Buglet in fseek?

[Jeff Johnston]

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.