[PATCH] D90759: Escape command line arguments in backtraces

Luke Drummond via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 13 09:38:08 PST 2020


ldrumm added a comment.

I absent mindedly responded to this review via email, without actually checking whether Phabricator supports this. Apologies for the delay



================
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], ' ');
----------------
jhenderson wrote:
> 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 << ' ';
+  }
----------------
jhenderson wrote:
> 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



More information about the llvm-commits mailing list