[Lldb-commits] [PATCH] D130098: [LLDB][NFC][Reliability] Fix uninitialized variables from Coverity scan

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4685
+    lldb::tid_t m_thread_id = LLDB_INVALID_THREAD_ID;
+    uint32_t m_thread_index = 0;
     std::string m_thread_name;
----------------
clayborg wrote:
> jingham wrote:
> > Because of the way CommandObjects work, the constructed values of its ivars are never used.  You have to call OptionParsingStarting before doing any work with the object, so the values set in that function are the real ones.  There's no actual point in initializing these variables, but it mostly doesn't hurt except if you set them to different values from the ones in OptionParsingStarting, in which case they could cause confusion in people reading the code.  Here you've set m_thread_index to 0 but the correct starting value is actually UINT32_MAX as you can see from the OptionParsingStarting just above.
> > 
> > Can you go through all of the CommandObject subclass ivars that you've given values to and make sure they are the ones in the class's OptionParsingStarting?  Thanks.
> yeah, I would like to get everything initialized if we can to limit our static analysis warnings.
If we just called OptionParsingStarting in the constructor would your static analysis tools be smart enough to see that they were initialized?  That's the initialization that actually matters so it would be better not to have to write the values in two places (one of which looks like it matters but actually doesn't...)


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:281
   std::string path_str = path.str();
-  bool success 
-      = CPlusPlusLanguage::ExtractContextAndIdentifier(path_str.c_str(),
-                                                       context,
-                                                       identifier);
+  bool success = CPlusPlusLanguage::ExtractContextAndIdentifier(
+      path_str.c_str(), context, identifier);
----------------
If you use 

llvm-project/clang/tools/clang-format/git-clang-format

to clang-format your changes, then it will only apply to your diff, and you won't pick up spurious changes like this that make diffs harder to read.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130098/new/

https://reviews.llvm.org/D130098