[PATCH] D109972: Regenerate LC_CODE_SIGNATURE during llvm-objcopy operations

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 20 00:46:11 PDT 2021


jhenderson added a comment.

Some stylistic comments. I'll leave @alexander-shaposhnikov to do the real review.



================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.h:21
   Object &O;
+  const StringRef OutputFilename;
   bool Is64Bit;
----------------
We don't usually bother marking member variables as `const` in the llvm-objcopy code, so I'd drop the `const` here and in the constructor parameter argument.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOWriter.cpp:10
 #include "MachOWriter.h"
+#include "../CommonConfig.h"
 #include "MachOLayoutBuilder.h"
----------------
You should be able to include this as in the inline edit (cf ELF/ELFObjcopy.cpp which doesn't have the relative include). Same goes elsewhere.


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:101
 
+  // Returns the segment fileoff if the load command is a segment command
+  Optional<uint64_t> getSegmentFileOffset() const;
----------------
Nit: comments must end in a full stop. Same in the two other cases below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109972



More information about the llvm-commits mailing list