[PATCH] D22693: More strongly separate the PDB reading interfaces and PDB writing interfaces

[Adrian McCarthy via llvm-commits]

amccarth added inline comments.

================
Comment at: lib/DebugInfo/Msf/MappedBlockStream.cpp:191
@@ -162,4 +190,3 @@
   uint32_t NumAdditionalBlocks =
-      llvm::alignTo(Size - BytesFromFirstBlock, Msf.getBlockSize()) /
-      Msf.getBlockSize();
+      llvm::alignTo(Size - BytesFromFirstBlock, BlockSize) / BlockSize;
 
----------------
zturner wrote:
> majnemer wrote:
> > amccarth wrote:
> > > Is this correct?  If `Size` is two blocks and `BytesFromFirstBlock` is less than `BlockSize`, then we'll get two additional blocks instead of one.
> > Perhaps:
> >   uint32_t NumAdditionalBlocks = (llvm::alignTo(Size, BlockSize) / BlockSize) - 1;
> > ?
> Are you sure?  Here's a concrete example:
> 
> ```
> // Starting from the beginning of block 2, read 2 full blocks of data.
> BlockSize = 4096
> Offset = 8192
> Size = 8192
> 
>   // BlockNum = 8192 / 4096 = 2;
>   uint32_t BlockNum = Offset / BlockSize;
> 
>   // OffsetInBlock = 8192 % 4096 = 0;
>   uint32_t OffsetInBlock = Offset % BlockSize;
> 
>   // BytesFromFirstBlock = min(8192, 4096 - 0) = 4096
>   uint32_t BytesFromFirstBlock = std::min(Size, BlockSize - OffsetInBlock);
> 
>   // NumAdditionalBlocks = alignTo(8192 - 4096, 4096) / 4096 = 4096 / 4096 = 1
>   uint32_t NumAdditionalBlocks =
>       llvm::alignTo(Size - BytesFromFirstBlock, BlockSize) / BlockSize;
> ```
> 
> 
I didn't understand what `BytesFromFirstBlock` was measuring.  Now that I do, I think the original code is correct.


https://reviews.llvm.org/D22693