[Lldb-commits] [PATCH] D33167: Get rid of some uses of StringConvert and reduce some indentation

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon May 15 05:58:51 PDT 2017

labath added a comment.

I think this is a step in the right direction. Besides reducing boilerplate, this will also help us ensure correctness, as we get a constant trickle of bug reports for commands which forgot to set the result status.

The name is not ideal, but it's probably the best we can get. (The ideal solution for me would be to get rid of the duality in the DoExecute function -- e.g. remove the bool return and let the execution state be fully signalled by the result object -- but that's way off from what you were originally trying to do).

That said, I'm not sure I should be the person approving this. :)

Comment at: lldb/source/Commands/CommandObjectPlatform.cpp:611
+    if (!platform_sp->CloseFile(fd, error))
+      return result.AppendError(eReturnStatusFailed, error.AsCString());
I believe this should be `"{0}",  error` to avoid hitting problems in case the error contains '{'. (Plus it's 5 chars shorter :) )

Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1181
+                                "Failed to send signal {0}: {1}\n", signo,
+                                ST.AsCString());
AsCString unnecessary


More information about the lldb-commits mailing list