[PATCH] D33180: Add functionality to cvtres to parse all entries in res file.

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/include/llvm/Object/ResFile.h:68
+private:
+  friend class ResEntryRef;
+};
----------------
zturner wrote:
> I think you should store the `BinaryByteStream` here in this class, not in the `Ref` class.  This makes a lot of things much easier.  See below for an example.
I made some improvements to the `BinaryStreamClass` recently, it should simplify some of this.


================
Comment at: llvm/include/llvm/Object/ResFile.h:82-87
+  // DWORD align
+  if (Offset % 4 != 0)
+    Offset += 4 - (Offset % 4);
+
+  // correct for the fact we already read 2 integers.
+  Offset -= 2*sizeof(uint32_t);
----------------
zturner wrote:
> Ideally we'd like to get rid of as much math as possible.  `BinaryStreamWriter` has a function called `padToAlign` that does exactly this.  Perhaps you could add `alignTo(unsigned N)` to `BinaryStreamReader which simply skips the appropriate number of bytes.
`BinaryStreamReader` now has a method called `padToAlignment`.  The entire rest of this function can just be replaced with:

```
RETURN_IF_ERROR(Reader.padToAlignment(sizeof(uint32_t));
End = (Reader.bytesRemaining() == 0);
```


================
Comment at: llvm/lib/Object/ResFile.cpp:37-38
+ResEntryRef ResFile::HeadEntry() {
+  size_t LeadingSize = sizeof(ResourceMagic) + sizeof(NullEntry);
+  ArrayRef<uint8_t> Ref(reinterpret_cast<const uint8_t*>(Data.getBufferStart() + LeadingSize), Data.getBufferSize() - LeadingSize);
+  return ResEntryRef(Ref, this);
----------------
zturner wrote:
> zturner wrote:
> > If the `BBS` is in the `ResFile` instead of the `ResFileRef`, and the `ResFileRef` instead stores a `BinaryStreamRef`, you can just write:
> > 
> > `return ResEntryRef(BBS.slice(LeadingSize, BBS.size()-LeadingSize));`
> Alternatively, `return ResEntryRef(BBS.drop_front(LeadingSize).drop_back(LeadingSize));`
My bad, I misunderstood this code the first time.  It's just `return ResEntryRef(BBS.drop_front(LeadingSize));`


https://reviews.llvm.org/D33180