[PATCH] D90759: Escape command line arguments in backtraces

[Luke Drummond via llvm-commits]

On Fri Nov 6, 2020 at 2:04 PM GMT, James Henderson via Phabricator wrote:

> jhenderson added a comment.
>
> Seems reasonable overall. Just a coupel of nits.
>
>
>
> ================
> Comment at: llvm/lib/Support/PrettyStackTrace.cpp:257
> // Print the argument list.
> - for (unsigned i = 0, e = ArgC; i != e; ++i)
> - OS << ArgV[i] << ' ';
> + for (int I = 0; I < ArgC; ++I) {
> + const bool HaveSpace = ::strchr(ArgV[I], ' ');
> ----------------
> What's the reason for changing the type of `I`? It's being used as an
> incrementing array index, so size_t seems like the most appropriate
> thing, if you're going to change it at all.
>
I changed it mostly because ArgC is signed, and signed / unsigned
comparisons make some people (and compilers) unhappy. The bounds of the
number of programming arguments are strictly within the range of an int,
so I don't think `size_t` buys anything.
>
> ================
> Comment at: llvm/lib/Support/PrettyStackTrace.cpp:264
> + OS << '"';
> + OS << ' ';
> + }
> ----------------
> It would be nice to not have this trailing space after the last
> argument.
Agreed. I'll fix this up
>
>
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D90759/new/
>
> https://reviews.llvm.org/D90759