On Wed, Feb 15, 2017 at 5:54 AM Pavel Labath via Phabricator <
revi...@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
>
>
>
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
