jhenderson added inline comments.
================
Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:567-568
+ compression::zstd::compress(
+ StringRef(reinterpret_cast<const char *>(OriginalData.data()),
+ OriginalData.size()),
+ CompressedData);
----------------
This StringRef construction is identical in both parts of the if, so should be
pulled out of the if statement into a local variable.
================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:1009-1019
+ if (Config.CompressionType != DebugCompressionType::Zstd) {
+ if (Config.DecompressDebugSections && !compression::zlib::isAvailable())
+ return createStringError(
+ errc::invalid_argument,
+ "LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress");
+ } else {
+ if (Config.DecompressDebugSections && !compression::zstd::isAvailable())
----------------
This is identical (I think?) to a block further up in the file. Could we pull
it out into a helper function, please? Something like the following:
```
Error checkCompressionAvailability() {
/* moved code here */
}
// Usage would look like:
if (Error err = checkCompressionAvailability())
return err;
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128667/new/
https://reviews.llvm.org/D128667
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
