Re: Buglet in fseek?

[Jeff Johnston <jjohnstn at redhat dot com>]

Corinna Vinschen wrote:
> Hi,
>
> while investigating a problem with top(1) on Cygwin, I came across this
> description of fseek(3) in SUSv3:
>
> http://www.opengroup.org/onlinepubs/009695399/functions/fseek.html
>
> Quote:
>
> "[CX] If the most recent operation, other than ftell(), on a given
>  stream is fflush(), the file offset in the underlying open file
>  description shall be adjusted to reflect the location specified by
>  fseek()."
>
> [CX] indicates an extension to the ISO C standard.  However, this is
> quite interesting behaviour.  Consider a file which contents are
> constantly changed, for instance by the OS.  In case of top(1), this
> would be /proc/stat or any other file within /proc which contains a
> snapshot of the current system state.
>
> As for top(1), what it does is this, basically:
>
>   if (!fp)
>     fp = fopen ("/proc/stat", "r");
>   rewind (fp);
>   fflush (fp);
>   fread (fp);
>
> This works only for the first time the function is called.  For each
> later call, rewind does not call the underlying system function lseek,
> which would refresh the contents of the file, because fseek_r optimizes
> the call away in case of streams only opened for reading.
>
> There's nothing wrong with that.  However, according to the above quote,
> one would think it should be possible to get the desired behaviour by
> just reordering the calls, so that fflush is called before rewind:
>
>   if (!fp)
>     fp = fopen ("/proc/stat", "r");
>   fflush (fp);
>   rewind (fp);
>   fread (fp);
>
> The above quote indicates that fseek called by rewind should always call
> lseek if the last action was a call to fflush.  But this is not the case
> in newlib so just reordering the calls doesn't work here, either.
>
> I admit this is not a real bug in newlib, but it's pretty unfortunate
> behaviour.  In newlib, there's no way to force an lseek in this
> situation by only using stream functions, except by either opening the
> file for writing, or by setting the stream buffering to unbuffered
> (setvbuf).  Of course you can always explicitely call lseek, but that
> seems somewhat unclean.
>
> Would it be a problem to implement the above quoted behaviour in newlib?

I have no objection to it; it is also mentioned in the POSIX spec.  It 
will be a little tricky to add to the current logic.

There are no spots left for flags in the _flags field of a FILE struct, 
however, there is already a flag in place (_SNPT) which tells fseek not 
to use the buffer optimization.  At present, the flag never gets reset 
anywhere.

One possible solution might be to have fflush set the _SNPT flag as well 
as saving the offset of the file (including read buffer position, 
ignoring ungetc).  When a seek occurs, if dealing with a read-only file 
with _SNPT flag set on and saved position != -1, a check is made 
regarding the current position.  If the true position differs from the 
saved position, then a read has occurred and we are not restricted to 
performing the lseek.  In that case, we simply reset the _SNPT flag and 
do the regular logic.  Otherwise, we allow the "goto dumb" logic to 
occur and we reset the _SNPT flag and saved position if appropriate.

What do you thin?  Have I missed anything?  Typical read-only file usage 
won't be interfered with.  There won't be any performance penalty added 
to reads/etc..

-- Jeff J.

> Corinna