[PATCH] D150305: ObjCopy: support `--dump-section` on COFF

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd marked 2 inline comments as done.
compnerd added inline comments.


================
Comment at: llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp:133
 
+static Error dumpSection(Object &O, StringRef S, StringRef File) {
+  for (const coff::Section &Section : O.getSections()) {
----------------
hjyamauchi wrote:
> nit: "S" -> "Section" to match "File"?
That collides with the iterator - I can use `SectionName` but that also is different from `File` ... I guess I can do `SectionName` and `FileName`.


================
Comment at: llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp:152
+  }
+  return createStringError(errc::no_such_file_or_directory,
+                           "section '%s' not found", S.str().c_str());
----------------
hjyamauchi wrote:
> nit: Would it be good to match the ELF version with "object_error::parse_failed"? Or the other way around? Perhaps doesn't matter.
Ugh!  That didn't match what I saw locally.  I guess I was out of date :-(.  Thanks!


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

https://reviews.llvm.org/D150305