[PATCH] D54384: [llvm-objcopy] Add --build-id-link-dir flag

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:140
+  sys::path::append(Path,
+                    llvm::toHex(ArrayRef<uint8_t>(BuildIdBytes->begin() + 1,
+                                                  BuildIdBytes->end()),
----------------
rupprecht wrote:
> jakehehrlich wrote:
> > mcgrathr wrote:
> > > Is there a check (and test coverage) for the error case where the build ID is less than two bytes?
> > This ArrayRef will be empty in that case. There is however no check to make sure that the build ID isn't empty so the [0] above could fail in theory.
> I agree -- this should error (with a test too :) ) if build id is less than 2 bytes
as David mentioned below - maybe return Expected<...> instead ? imho that would be more readable, no "Out" parameters, etc ?


Repository:
  rL LLVM

https://reviews.llvm.org/D54384