[PATCH] run the unit test for MemoryBuffer::getOpenFile in two different modes

[Rafael Espíndola]

>>Unless you want to test unbuffered files too, it might be better to do
>>something like
>>{
>> raw_fd_ostream OF(TestFD, true, true);
>>....
>>}
>>OwningBuffer Buf;
>
> I don't mind about buffering - really just wanted the results to be seen in
> the file immediately without closing it. The new patch explicitly separates
> the two cases: (1) file is still open and (2) file is close and then
> reopened for reading.

Yes, with the second patch we are good. One nit is that you might want
to open with buffering when it will be closed, to make sure buffers
are flushed.

>>You probably want to pass the buff size, not the filesize down to
>>MemoryBuffer::getOpenFile.
>
> You are probably right; I was following the usage of getOpenFile within
> gold-plugin.cpp, which deals with "files within files" and also passes the
> size of the sub-file into FileSize. Now looking more closely on that
> invocation, something doesn't make sense: When MapSize = -1, getOpenFile
> sets MapSize = FileSize. But then, shouldUseMmap asserts:

Oops. That is probably my bug. The FileSize being passed down should
be what stat would return. Maybe we should add an assert?

>   size_t End = Offset + MapSize;
>   assert(End <= FileSize);
>
> Meaning that if a positive offset is passed, and MapSize = 1 this assertion
> would fire (unless shouldUseMmap bails out for small sizes).

You mean MapSize == -1, right? But yes, shouldUseMmap should return
false for small map sizes, even if they are in large files.

> So is gold-plugin.cpp using the API incorrectly? Note that actually using
> this Offset is uncommon, ISTM only gold/lto related code uses it.
>
> [in any case it would probably be beneficial to explain the API in more
> detail in a comment - I can do that with my next commit there]

Do you want me to try to add the assert?

> Eli

Cheers,
Rafael