[PATCH] D63395: [llvm-objcopy][MachO] Support load commands used in executables/shared libraries

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOWriter.h:28
+  // Points to the __LINKEDIT segment if it exists.
+  MachO::macho_load_command *LinkEditLoadCommand = nullptr;
 
----------------
so after some thinking about what's going on here it seems to me
maybe it's possible to move the logic for building the layout from MachOWriter into a separate class MachOLayoutBuilder, basically it's motivated by "separation of concerns" / desire to simplify a bit what's going on here. Roughly speaking the idea is the following:  

1. MachOLayoutBuilder will take the model (Object) and update all the offsets / sizes. In particular, it might hold some additional state internally (like MachO::macho_load_command *LinkEditLoadCommand) and will be responsible for calling the private subroutines (computeSizeOfCmds, layoutSegments, layoutRelocations, updateDySymTab, layoutTail) in the correct order, etc. The contract here is that after calling its public method build() the layout is final and one can safely use it to calculate / get the total size of the binary, all the offsets/sizes are valid etc. 

2. MachOWriter will be responsible for managing the Buffer + writing things into it properly, but it will never modify the model (Object) directly (but internally it can make use of MachOLayoutBuilder before writing things out).

What do you think ?



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

https://reviews.llvm.org/D63395