[PATCH] D61993: [llvm-objcopy] Add file names to error messages

[Seiya Nuta via Phabricator via llvm-commits]

seiya marked 3 inline comments as done.
seiya added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:688
     if (Error E = dumpSectionToFile(SecName, File, Obj))
-      return createFileError(Config.InputFilename, std::move(E));
+      return createFileError(File, std::move(E));
   }
----------------
jhenderson wrote:
> I don't have a problem with this change, but why did you make it? Also, it should probably be a separate patch, since it's not related to the bug you are trying to fix.
I changed this to print the output file name specified in --dump-section because, for instance, creating the output file named `File` may fail with an EISDIR error.

I'll remove this line and submit another patch to update this.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:737
   if (!Config.BuildIdLinkDir.empty()) {
-    BuildIdBytes = unwrapOrError(findBuildID(In));
+    auto BuildIdBytesOrErr = findBuildID(In);
+    if (auto E = BuildIdBytesOrErr.takeError())
----------------
jhenderson wrote:
> Rather than create the file error here, it might make more sense to call createFileError in findBuildId.
Indeed. I'll change findBuildId to take Config and return a FileError from it. 


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:751
     if (Error E =
             linkToBuildIdDir(Config, Config.InputFilename,
                              Config.BuildIdLinkInput.getValue(), BuildIdBytes))
----------------
jhenderson wrote:
> linkToBuildIdDir already calls createFileError in some situations. Rather than repeat it here, it would be better to change any necessary return sites in that function.
I thought it would be helpful to print the input file name but now realized that it is not so helpful. I'll remove this change and return the error from this function as it is because it already includes the file name which caused the error.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61993