[PATCH] COFFDumper: Dump data directory entries.

[Rafael Espíndola]

> Size may be from an input file so it can be a very large number. I think the extra condition is to check for integer overflow. We shouldn't use assert() here because if it fails it means the input file is broken and it's not a bug of this code.

We don't have special cases for 64 bit integers overflowing in other
parts of the codebase. You are right, it cannot be an assert. It would
be interesting to have some form of fatal error instead, but you don't
need to do that in this patch.

> ================
> Comment at: lib/Object/COFFObjectFile.cpp:613
> @@ +612,3 @@
> +                                            const data_directory *&Res) const {
> +  // Error if if there's no data directory or the index is out of range.
> +  if (!DataDirectory || index > PE32Header->NumberOfRvaAndSize)
> ----------------
> Rafael Ávila de Espíndola wrote:
>> Just returing a data_directory* (possibly null) would be a simpler interface.
> All the other get* functions have the same return type. I think it's nice to keep it consistent with others.

 A good part of this API has a way more general interface than what is
needed. Returning an error_code to me at least gives some wrong
impressions:

* The getter is doing actual work (like parsing), and can really fail.
* There are multiple possible ways it can fail.

Since it is the constructor that is doing the actual work, just
returning a pointer is preferable. Getters like these are fairly
common in LLVM.

Cheers,
Rafael