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

[Zachary Turner via lldb-commits]

Indeed, ideally there is no bool return value and we just write return
make_error<Failure>("foo")
On Mon, May 15, 2017 at 5:58 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170515/0f303647/attachment.html>