[llvm] r208007 - [Support/MemoryBuffer] Introduce a boolean parameter (false by default) 'IsVolatile' for the open file functions.

[Argyrios Kyrtzidis]

On May 5, 2014, at 4:21 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:

>> -  if (shouldUseMmap(FD, FileSize, MapSize, Offset, RequiresNullTerminator,
>> +  if (!IsVolatile &&
>> +      shouldUseMmap(FD, FileSize, MapSize, Offset, RequiresNullTerminator,
>>                     PageSize)) {
> 
> Why not put this in shouldUseMmap itself?

Will do that.

> 
> A more serious concern is that the semantics are pretty fuzzy. What
> exactly does it mean to change often? If we are at risk of reading
> modified data by using mmap, how do we guarantee that we don't read
> modified data with read? Is the caller required to hold a lock when
> doing the call? If so, that should be a comment saying so.

The problem is not the data itself, the problem is that mmap may leave the buffer without a null terminator, because the file size changed by the time mmap brings in the last page.
Initializing the memory with read/pread has no such issue, the null terminator is guaranteed.

> 
> What is the intended use case for this?

libclang passes isVolatile = true for user files, because it has to deal with user files constantly getting edited/updated while trying parse them.

> 
> Cheers,
> Rafael