Re: Buglet in fseek?

[Jeff Johnston <jjohnstn at redhat dot com>]

Jeff Johnston wrote:
> 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.

I should haved clarified that the new fflush logic is only for read-only 
files; writable files do not have the fflush/fseek problem cited. 
Resetting the saved position is akin to setting it to -1 (an invalid 
value).  The FILE struct would have to be altered to add the new area to 
save the offset.

-- Jeff J.