[PATCH] D157210: [symbolizer] Change reaction on invalid input

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Just some nits left from me.



================
Comment at: llvm/test/tools/llvm-symbolizer/get-input-file.test:4
+
+# No input itemms at all, - complain about missing input file.
+RUN: echo | llvm-symbolizer 2>%t.1.err | FileCheck %s --check-prefix=NOSOURCE
----------------



================
Comment at: llvm/test/tools/llvm-symbolizer/get-input-file.test:8
+
+# Only one input item, - complain about missing addresses.
+RUN: llvm-symbolizer "foo" 2>%t.2.err | FileCheck %s --check-prefix=NOSOURCE
----------------



================
Comment at: llvm/test/tools/llvm-symbolizer/get-input-file.test:28
+
+# File name is an unterminated string.
+RUN: llvm-symbolizer "FILE:\"foo" 2>%t.7.err | FileCheck %s --check-prefix=NOSOURCE
----------------
This comment shoudl say something about unbalanced quotes, not "unterminated string", which implies something about a missing `\0` to me.


================
Comment at: llvm/test/tools/llvm-symbolizer/get-input-file.test:48
+BAD-QUOTE: error: 'FILE:"foo': unbalanced quotes in input file name
\ No newline at end of file

----------------
Nit: no new line at EOF.


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:193
 
+  // If input file is not specified in command line, try to extract it from
+  // the command.
----------------



================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:222
+    if (ModuleName.empty())
+      return makeStringError("no input filename is specified");
   }
----------------
"is" -> "has been"


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:228
+  if (InputString.empty())
+    return makeStringError("no module offset is specified");
   int OffsetLength = InputString.find_first_of(" \n\r");
----------------
"is" -> "has been"


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:195
     if (!BinaryName.empty() || !BuildID.empty())
-      // Input file has already been specified on the command line.
-      return false;
+      return makeStringError("input file is already specified");
     StringRef Name = getSpaceDelimitedWord(InputString);
----------------
jhenderson wrote:
> 
My apologies, this was stupid of me. It should be "input file has already **been** specified"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157210/new/

https://reviews.llvm.org/D157210