[Lldb-commits] [PATCH] D29964: Finish breaking the dependency from lldbUtility -> Host

[Zachary Turner via lldb-commits]

On Wed, Feb 15, 2017 at 5:54 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> labath added inline comments.
>
>
> ================
> Comment at: lldb/source/Utility/VASprintf.cpp:18
> +                             va_list args) {
> +  llvm::SmallString<16> error("<Encoding error>");
> +
> ----------------
> It doesn't look like you should need to allocate a stack object with the
> string every time. Can't you just assign `buf = "..."` in the error case?
>
>
No because buf is a SmallVectorImpl, which doesn't have a conversion from
anything else



>
> ================
> Comment at: lldb/source/Utility/VASprintf.cpp:28
> +  if (length < 0) {
> +    buf = error;
> +    goto finish;
> ----------------
> Could you also have the function return false in case of an error? You can
> leave the callers ignoring it, as they weren't handling these errors
> anyway, but now at least they would have a chance. I know this is meant to
> go away in the future, but noone know how soon will that be, and it seems
> like a bad idea to introduce a new abstraction with such an obviously
> flawed interface (as in, the only way to check if it failed is to inspect
> the printed-to string).
>
>
> https://reviews.llvm.org/D29964
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170215/30250f59/attachment-0001.html>