[PATCH] D124607: Add an error message to the default SIGPIPE handler

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-cxxfilt/unix03-sigpipe-exit.test:1
+# Test that when nm tries to write to a closed stdout it will finish with
+# a non-zero exit code and an error message on stderr.
----------------
Comment needs updating.

I actually think the test as a whole should be moved to llvm/test/Support, since it's really testing a generic piece of behaviour, rather than nm's/cxxfilt's behaviour.

Unrelated, but I often encourage using double comment markers (i.e. `##`) for true comments, versus lit/FileCheck directives, so that they stand out better.


================
Comment at: llvm/test/tools/llvm-cxxfilt/unix03-sigpipe-exit.test:23
+  except BrokenPipeError:
+    # Clear stdin, pipe is broken and closing it on cleanup will rise an exception 
+    process.stdin = None
----------------



================
Comment at: llvm/test/tools/llvm-cxxfilt/unix03-sigpipe-exit.test:25
+    process.stdin = None
+    pass
+sys.exit(process.returncode)
----------------
No need for `pass` when there's a statement here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124607