This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] Use cached offset in ftell when reliable


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/04/2014 02:05 AM, Siddhesh Poyarekar wrote:
>>> +
>>> +      /* It is safe to use the cached offset when available if there is
>>> +	 unbuffered data (indicating that the file handle is active) and the
>>> +	 handle is not for a file open in a+ mode.  The latter condition is
>>> +	 because there could be a scenario where there is a switch from read
>>> +	 mode to write mode using an fseek to an arbitrary position.  In this
>>> +	 case, there would be unbuffered data due to be appended to the end of
>>> +	 the file, but the offset may not necessarily be the end of the
>>> +	 file.  It is fine to use the cached offset when the a+ stream is in
>>> +	 read mode though, since the offset is maintained correctly in that
>>
>> Just to be clear this is because we rely on O_APPEND being handled in the
>> kernel? If we implemented O_APPEND in userspace, like the kernel does for
>> NFS, it would not reliably append to the file in the presence of multiple
>> writers, *but* we would have an accurate cached offset in the library.
> 
> I didn't know that O_APPEND was done in userspace for NFS.

All I wanted to say was that the NFS protocol does not support atomic
append writes. The actual NFS implementation can be anywhere. Please
ignore this NFS comment as it isn't really relevant to the problem at
hand.

I only meant to say that glibc could emulate O_APPEND and have an
accurate cached file offset, but that would break atomic append
writes (like NFS is broken).

> Even then our cached offset would not always be correct unless we
> explicitly try to make it so.  This would be seen when we switch from
> read mode to write mode on an a+ file.  Unless we explicitly move the
> file descriptor offset to the end of file using lseek and refresh the
> offset cache, we would see the offset as being somewhere in the middle
> of the file instead of at the end of the file.

Correct.
 
>>> +	 case.  Note that this is not a comprehensive set of cases when the
>>> +	 offset is reliable.  The offset may be reliable even in some cases
>>> +	 where there is no buffered input and the handle is active, but it's
>>> +	 just that we don't have a way to identify that condition reliably.  */
>>
>> s/no buffered/unbuffered/g
> 
> I've used 'no unflushed input' since that describes the situation more
> accurately.

OK.

>>> -  /* Set up initially to use the `maybe_mmap' jump tables rather than using
>>> -     __fopen_maybe_mmap to do it, because we need them in place before we
>>> -     call _IO_file_attach or else it will allocate a buffer immediately.  */
>>
>> Why are you removing this? Because we remove the call to _IO_file_attach?
> 
> Yes, the comment didn't seem relevant anymore.  Using the maybe_mmap
> jump tables is still necessary - I haven't figured out why, but it
> broke during testing.

OK.

>> I don't follow this comment, could you expand on that a little.
>>
>> The offset and the fd are related in that you could get the former
>> from the latter, is that why we record it here? For later use?
> 
> The comment was badly worded.  I meant to say two things: first being
> that unsetting the offset is important so that other functions don't
> rely on it and the second being that the offset is already unset, so
> we just need to set the fd.  I have updated the comment to (hopefully)
> be a bit clearer.

That's perfect. That clarifies it for me. Thanks.

>> Try to leave out formatting changes.
> 
> Ack, I've split it into a different commit.
> 
> This is what I have finally committed.

Looks good.

Cheers,
Carlos.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTFYGUAAoJECXvCkNsKkr/bVoH/j5XXlyw6hvL2DTdDS8nQsHr
DGyx60B1Vp50R3gDTcfDP9hkb1USaZCEhMQKOJ4Oi3whgM7QuRmC1KWypUxbaG/9
/yAbGMCQKHXshlN1cn72f4HvNSQosBGa4atUvdiL1mlLiRYfQbd/Tu2SOed18hIS
8MkXhUpoQtxxByOBGZxyvu6l13IrE8DaijAIHwLnfTTwON7LP8x8kbjGOhajQdJl
NLAxPJBAqgW+pQV++koEaSEzMW+h1xrJAyvl60TH+38Rn7VwXTXI/Rmin9AQlQd8
8lJCoFjOnWRcB5/bC3JlSC9TFt0wcBVWHfXxCKhHPx8+1J/EtTmnJRVSzTCaVY4=
=xZqb
-----END PGP SIGNATURE-----


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]