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
https://reviews.llvm.org/D33167
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
