Address code reviews

llvm · Feb 3, 2025 · 6f2a030 · 6f2a030
1 parent a13c4c9
commit 6f2a030
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 36 deletions.
diff --git 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 <typename... Ts>
+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
@@ -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<std::unique_ptr<FileOutputBuffer>> 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,37 +828,32 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig,
         if (Config.ChangeSectionLMAValAll > 0 &&
             Seg.PAddr > std::numeric_limits<uint64_t>::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<uint64_t>::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;
       }
     }
   }
 
   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<AddressUpdate> 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<uint64_t>::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
@@ -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
@@ -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) {