[PATCH] libio: Implement internal function __libc_readline_unlocked

[Florian Weimer]

On 07/06/2018 02:31 PM, Florian Weimer wrote:
> On 11/22/2017 06:50 AM, Jonathan Nieder wrote:
>> Florian Weimer wrote:
>>
>>> --- a/libio/ftello.c
>>> +++ b/libio/ftello.c
>>> @@ -61,4 +61,6 @@ weak_alias (__ftello, ftello)
>>>
>>> Â  #ifdef __OFF_T_MATCHES_OFF64_T
>>> Â  weak_alias (__ftello, ftello64)
>>> +strong_alias (__ftello, __ftello64)
>>
>> What does this part do?
> 
> If off_t and off64_t are the same type, __ftello and __ftello64 are the 
> same function (at the same address).
> 
>> [...]
>>> +++ b/libio/readline.c
>>> @@ -0,0 +1,169 @@
>> [...]
>>> +/* Slow path for reading the line.  Called with no data in the stream
>>> +   read buffer.  Write data to [BUFFER, BUFFER_END).  */
>>> +static ssize_t
>>> +readline_slow (_IO_FILE *fp, char *buffer, char *buffer_end)
>>> +{
>>> +Â  char *start = buffer;
>>> +
>>> +Â  while (buffer < buffer_end)
>>> +Â Â Â  {
>>> +Â Â Â Â Â  if (__underflow (fp) == EOF)
>>> +Â Â Â Â Â Â Â  {
>>> +Â Â Â Â Â Â Â Â Â  if (_IO_ferror_unlocked (fp))
>>> +            /* If the EOF was caused by a read error, report it.  */
>>> +Â Â Â Â Â Â Â Â Â Â Â  return fail_no_erange ();
>>
>> Is readline_slow's caller responsible for ensuring !ferror(fp) before
>> the __underflow call?  Otherwise it's possible for this to return -1
>> without setting errno.
> 
> The caller should not call the function if the stream is in an error 
> condition I added a comment to the header file.
> 
>>
>>> +Â Â Â Â Â Â Â Â Â  *buffer = '\0';
>>> +          /* Do not include the NULL terminator.  */
>>
>> nit: s/NULL/NUL/ or s/NULL/null/ (NULL is a pointer, NUL an ascii code
>> point).
> 
> Thanks, fixed.
> 
>> [...]
>>> +  /* Slow path: Read more data from the underlying file.  We need to
>>> +     restore the file pointer if the buffer is too small.  First,
>>> +     check if the __ftello64 call above failed.  */
>>> +Â  if (start_offset < 0)
>>> +Â Â Â  return fail_no_erange ();
>>> +
>>> +Â  ssize_t result = readline_slow (fp, buffer, buffer_end);
>>> +Â  if (result < 0)
>>> +Â Â Â  {
>>> +Â Â Â Â Â  if (errno == ERANGE)
>>> +Â Â Â Â Â Â Â  {
>>> +Â Â Â Â Â Â Â Â Â  /* Restore the file pointer so that the caller may read the
>>> +             same line again.  */
>>> +Â Â Â Â Â Â Â Â Â  if (__fseeko64 (fp, start_offset, SEEK_SET) < 0)
>>> +Â Â Â Â Â Â Â Â Â Â Â  return fail_no_erange ();
>>
>> What happens if the file is unseekable?  Is this function not meant to
>> be used for such files, or is this seeking to reset the file pointer a
>> best-effort kind of thing?
> 
> The function is not meant to be used for such files.
> 
> I've rebased the patch and fixed the internal alias generation logic.
> 
> Carlos, is this and the fix for bug 18991 (which has already been 
> reviewed by DJ) still acceptable during the freeze?
> 
> Thanks,
> Florian

And now with the patch attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: readline.patch
Type: text/x-patch
Size: 20005 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20180706/1d271218/attachment.bin>