[PATCH] D25356: Define PDBFileBuilder::addStream to add stream.
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 10 15:12:19 PDT 2016
ruiu added inline comments.
================
Comment at: lib/DebugInfo/PDB/Raw/DbiStreamBuilder.cpp:49
+ ArrayRef<uint8_t> Data) {
+ auto ExpectedIndex = Msf.addStream(Data.size());
+ if (!ExpectedIndex)
----------------
zturner wrote:
> I think we shouldn't call `addStream` here. Suppose someone called `addDbgStream` more than once for the same value of `Type`. Then you would end up with unreferenced streams in the file. Maybe instead we can just store the `ArrayRef`, overwriting any previous value of the `ArrayRef` that was there before. Then in `finalizeMsfLayout` we can add all the streams.
>
> This way everything is semantically "atomic". No modification to the underlying file happens until you call `finalize()`, so you can do whatever you want until then without risk of messing anything up.
How about this? I think this is atomic because we check everything that can fail before calling addStream in this function.
https://reviews.llvm.org/D25356
More information about the llvm-commits
mailing list