[PATCH] D88827: [llvm-objcopy][NFC] Move core implementation of llvm-objcopy into separate library.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

> Using a stream approach sounds reasonable. I don't really know what the benefits are of using a memory mapped file versus other options (I vaguely recall from some older work that they improve performance, but am not sure if that is still the case or not). The one concern I'd have with a stream for writing output is if we ever need to jump back and forth within the object for some reason. Without looking at the existing objcopy code, I don't know if there are any instances where this happens though.

If there is such necessity then instead of raw_ostream there could be used raw_pwrite_stream, which allows such seek&update functionality.

During this refactoring effort, I am not going to rewrite the existing objcopy writing code.
So I plan to do things this way:

  Error executeObjcopyOnBinary(Config,In, raw_ostream &Out) {
    // TODO: refactor "writing" code to output into "raw_ostream &Out"
    // TOFO: directly, without MemBuffer in the middle.
    MemBuffer.allocate(Size)
    // existing writing code.
    MemBuffer.commit();
    Out.write(MemBuffer.getBufferStart(), MemBuffer.getBufferSize());
  }

I propose to use raw_ostream now and replace it with raw_pwrite_stream later, if it would be necessary.
If it would be hard to use raw_ostream during such postponed rewriting then we could change raw_ostream into raw_pwrite_stream at that moment.

If it is already clear that we need to use raw_pwrite_stream then I will use it within my patch.

> By the way, I think the preallocate method might be better termed reserve as it sounds like it solves a similar intent as std::vector::reserve to me.

Ok


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88827