[PATCH][BZ #5298] Don't flush write buffer for ftell

[Siddhesh Poyarekar]

On Mon, 24 Sep 2012 10:49:58 -0600, Jeff wrote:
> That's how I typically see that formatted for GCC.  That'll also
> cause a minor formatting change to the comment and return EOF line.
> Similarly for the wfileops.c implementation.  If glibc's style
> guidelines are different, then please ignore my comments.

My mistake. I don't remember what I was thinking when I formatted it
that way, but it is obviously wrong.

> The normal version is quite simple; the wide version is considerably 
> more complex.  Did you do any additional testing on the wide version 
> beyond what's already in the glibc testsuite?

The wide version has the additional overhead of conversion to find the
actual offset, but other than that the logic should be the same.  I
tested this with the test cases in the wide fseek patch, which tests
cases of ftell after seeking to various places as well as writing to
the stream. The tst-rewind* test cases in the testsuite should test
ftell after rewind.

> NEWS file will need 5298 added to the list of bugs fixed.

Will do on commit.

> In fileops.c:
> 
> +	  /* We're doing ftell and we're in write mode.  Add the
> unflushed
> +	     buffer contents to the file position and the amount by
> which the
> +	     file offset would have moved if the write had flushed.
> The
> +	     latter is usually non-positive.  */
> +	  offset += ((fp->_IO_write_ptr - fp->_IO_write_base)
> +		     + fp->_IO_write_base - fp->_IO_read_end);
> 
> I had to look at that several times.  The subtraction of _read_end
> from _write_base certainly looks odd.  You do something similar in 
> wfileops.c.  I wasn't able to convince myself it was correct, as long
> as you're confident it's correct, I won't object.

While writing an explanation for this, I realized that this could be
made simpler.

When fp._offset is set (which occurs when there has been a prior read),
_IO_read_end is always set to reflect its position in the buffer and
the buffer may look like this in write mode:

_IO_read_end,fp._offset,	_IO_write_ptr  
_IO_write_base			   |
	|			   |
	V			   V
    ------------------------------------
    |   |                          |   |
    ------------------------------------

We need the position at _IO_write_ptr.  Given fp._offset and the fact
that _IO_read_end coincides with it, this should be:

fp._offset - (_IO_read_end - _IO_write_ptr)

which is a simplified version of what I have coded in.  Another
interesting state for the buffer is:

    _IO_write_base  _IO_write_ptr  _IO_read_end, fp._offset
	|		|	       |
	V		V	       V
    ----------------------------------------
    |   |               |              |   |
    ----------------------------------------

This is when you have a read followed by a seek to somewhere in the
middle of the buffer, followed by a write.  Even in this case, the
above holds true.  The other case of only write is trivial since
_IO_write_base coincides with _IO_read_end and we end up with
unbuffered content, which can be added to the real offset of the file
that we get using lseek.

I have modified this section accordingly and also added a comment to
explain why _IO_read_end is there.  Updated patch attached.

Regards,
Siddhesh

ChangeLog:

	[BZ #5298]
	* libio/fileops.c (_IO_new_file_seekoff): Don't flush buffer
	for ftell.  Compute offsets from write pointers instead.
	* libio/wfileops.c (_IO_wfile_seekoff): Likewise.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: flushless-ftell.patch
Type: text/x-patch
Size: 6765 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20120925/a51574ab/attachment.bin>