From 6f2a030ca837bf2a691225aea1dca6e436d8ec5d Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Mon, 3 Feb 2025 18:51:10 +0100 Subject: [PATCH] Address code reviews --- llvm/include/llvm/Support/Error.h | 16 +++++++++ llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 47 +++++++++---------------- llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp | 5 ++- llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp | 5 ++- 4 files changed, 37 insertions(+), 36 deletions(-) diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h index 90120156ec2ea..f2d910a965991 100644 --- a/llvm/include/llvm/Support/Error.h +++ b/llvm/include/llvm/Support/Error.h @@ -1404,6 +1404,22 @@ inline Error createFileError(const Twine &F, size_t Line, std::error_code EC) { return createFileError(F, Line, errorCodeToError(EC)); } +/// Concatenate a source file path with a std::error_code and string error +inline Error createFileError(const Twine &F, std::error_code EC, + const Twine &S) { + Error E = createStringError(EC, S); + return createFileError(F, std::move(E)); +} + +/// Concatenate a source file path with a std::error_code and string error with +/// values +template +inline Error createFileError(const Twine &F, std::error_code EC, + char const *Fmt, const Ts &...Vals) { + Error E = createStringError(EC, Fmt, Vals...); + return createFileError(F, std::move(E)); +} + Error createFileError(const Twine &F, ErrorSuccess) = delete; /// Helper for check-and-exit error handling. diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp index 9d979a7c094d4..9c78f7433ad33 100644 --- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp +++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp @@ -189,13 +189,10 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename, StringRef InputFilename, Object &Obj) { for (auto &Sec : Obj.sections()) { if (Sec.Name == SecName) { - if (Sec.Type == SHT_NOBITS) { - Error E = - createStringError(object_error::parse_failed, - "cannot dump section '%s': it has no contents", - SecName.str().c_str()); - return createFileError(InputFilename, std::move(E)); - } + if (Sec.Type == SHT_NOBITS) + return createFileError(InputFilename, object_error::parse_failed, + "cannot dump section '%s': it has no contents", + SecName.str().c_str()); Expected> BufferOrErr = FileOutputBuffer::create(Filename, Sec.OriginalData.size()); if (!BufferOrErr) @@ -208,10 +205,9 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename, return Error::success(); } } - Error E = createStringError(object_error::parse_failed, - "section '%s' not found", SecName.str().c_str()); - return createFileError(InputFilename, std::move(E)); + return createFileError(InputFilename, object_error::parse_failed, + "section '%s' not found", SecName.str().c_str()); } Error Object::compressOrDecompressSections(const CommonConfig &Config) { @@ -832,24 +828,21 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, if (Config.ChangeSectionLMAValAll > 0 && Seg.PAddr > std::numeric_limits::max() - Config.ChangeSectionLMAValAll) { - Error E = createStringError( - errc::invalid_argument, + return createFileError( + Config.InputFilename, errc::invalid_argument, "address 0x" + Twine::utohexstr(Seg.PAddr) + " cannot be increased by 0x" + Twine::utohexstr(Config.ChangeSectionLMAValAll) + ". The result would overflow"); - return createFileError(Config.InputFilename, std::move(E)); } else if (Config.ChangeSectionLMAValAll < 0 && Seg.PAddr < std::numeric_limits::min() - Config.ChangeSectionLMAValAll) { - Error E = createStringError( - errc::invalid_argument, + return createFileError( + Config.InputFilename, errc::invalid_argument, "address 0x" + Twine::utohexstr(Seg.PAddr) + " cannot be decreased by 0x" + Twine::utohexstr(std::abs(Config.ChangeSectionLMAValAll)) + ". The result would underflow"); - - return createFileError(Config.InputFilename, std::move(E)); } Seg.PAddr += Config.ChangeSectionLMAValAll; } @@ -857,12 +850,10 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, } if (!Config.ChangeSectionAddress.empty()) { - if (Obj.Type != ELF::ET_REL) { - Error E = createStringError( - object_error::invalid_file_type, + if (Obj.Type != ELF::ET_REL) + return createFileError( + Config.InputFilename, object_error::invalid_file_type, "cannot change section address in a non-relocatable file"); - return createFileError(Config.InputFilename, std::move(E)); - } StringMap SectionsToUpdateAddress; for (const SectionPatternAddressUpdate &PatternUpdate : make_range(Config.ChangeSectionAddress.rbegin(), @@ -873,26 +864,22 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, .second) { if (PatternUpdate.Update.Kind == AdjustKind::Subtract && Sec.Addr < PatternUpdate.Update.Value) { - Error E = createStringError( - errc::invalid_argument, + return createFileError( + Config.InputFilename, errc::invalid_argument, "address 0x" + Twine::utohexstr(Sec.Addr) + " cannot be decreased by 0x" + Twine::utohexstr(PatternUpdate.Update.Value) + ". The result would underflow"); - - return createFileError(Config.InputFilename, std::move(E)); } if (PatternUpdate.Update.Kind == AdjustKind::Add && Sec.Addr > std::numeric_limits::max() - PatternUpdate.Update.Value) { - Error E = createStringError( - errc::invalid_argument, + return createFileError( + Config.InputFilename, errc::invalid_argument, "address 0x" + Twine::utohexstr(Sec.Addr) + " cannot be increased by 0x" + Twine::utohexstr(PatternUpdate.Update.Value) + ". The result would overflow"); - - return createFileError(Config.InputFilename, std::move(E)); } switch (PatternUpdate.Update.Kind) { diff --git a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp index 49ac903e99573..682edffc84f34 100644 --- a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp +++ b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp @@ -323,9 +323,8 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename, } } - Error E = createStringError(object_error::parse_failed, - "section '%s' not found", SecName.str().c_str()); - return createFileError(InputFilename, std::move(E)); + return createFileError(InputFilename, object_error::parse_failed, + "section '%s' not found", SecName.str().c_str()); } static Error addSection(const NewSectionInfo &NewSection, Object &Obj) { diff --git a/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp b/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp index 3ace39ac558f5..57fd0f5ad233c 100644 --- a/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp +++ b/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp @@ -53,9 +53,8 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename, return Error::success(); } } - Error E = createStringError(errc::invalid_argument, "section '%s' not found", - SecName.str().c_str()); - return createFileError(Filename, std::move(E)); + return createFileError(Filename, errc::invalid_argument, + "section '%s' not found", SecName.str().c_str()); } static void removeSections(const CommonConfig &Config, Object &Obj) {