getdelim(3): Fix losing data on EAGAIN

[bdrewery]

Summary

Currently when an [EAGAIN] is encountered we return a partial result
that does not contain the delimeter. On the next (successful) read we
were returning the next part of the line without the preceding string
from the first failed call.

Fix this by using the same mechanism as ungetc(3) does. For the buffered
case we could simply set fp->_r and fp->_p back to their values before
sappend() is ran but for simplicity ungetc(3) is done in there as well.

This, and a simpler test repro, were posted on an OpenBSD mailing list a few
years ago as well.
http://openbsd-archive.7691.n7.nabble.com/getdelim-and-O-NONBLOCK-td363434.html
http://webcache.googleusercontent.com/search?q=cache:jEH3oOsWpcAJ:openbsd-archive.7691.n7.nabble.com/getdelim-and-O-NONBLOCK-td363434.html+&cd=1&hl=en&ct=clnk&gl=us

Test Plan

# Without fix
getdelim_test:empty_NULL_buffer  ->  passed  [0.003s]
getdelim_test:eof  ->  passed  [0.003s]
getdelim_test:getline_basic  ->  passed  [0.003s]
getdelim_test:invalid_params  ->  passed  [0.003s]
getdelim_test:nonblock_eagain_buffered  ->  failed: /root/git/freebsd/main2/lib/libc/tests/stdio/getdelim_test.c:354: "" != line ( != first line partir)  [0.003s]
getdelim_test:nonblock_eagain_unbuffered  ->  failed: /root/git/freebsd/main2/lib/libc/tests/stdio/getdelim_test.c:354: "" != line ( != first line partir)  [0.003s]
getdelim_test:nul  ->  passed  [0.002s]
getdelim_test:stream_error  ->  passed  [0.002s]

Results file id is usr_tests_lib_libc_stdio.20210825-232552-052032
Results saved to /root/.kyua/store/results.usr_tests_lib_libc_stdio.20210825-232552-052032.db

6/8 passed (2 failed)

# With fix
getdelim_test:empty_NULL_buffer  ->  passed  [0.003s]
getdelim_test:eof  ->  passed  [0.003s]
getdelim_test:getline_basic  ->  passed  [0.003s]
getdelim_test:invalid_params  ->  passed  [0.002s]
getdelim_test:nonblock_eagain_buffered  ->  passed  [0.004s]
getdelim_test:nonblock_eagain_unbuffered  ->  passed  [0.004s]
getdelim_test:nul  ->  passed  [0.002s]
getdelim_test:stream_error  ->  passed  [0.003s]

Results file id is usr_tests_lib_libc_stdio.20210825-231537-629982
Results saved to /root/.kyua/store/results.usr_tests_lib_libc_stdio.20210825-231537-629982.db

8/8 passed (0 failed)

[bdrewery]

• Make failure of the partner process less prominent in kyua output

[kib]

I believe stdio is not supposed to work (correctly) with non-blocking file descriptors. I do not object against attempts to make part of it work regardless, but I am not sure it is possible without making it all functional.

lib/libc/stdio/getdelim.c

143–144

You can reduce indent for if (errno == EAGAIN) block if change this line into

if (__sfeof(fp))
   goto done; /* hit EOF */

144

I suspect it is possible for __srefill() to return EOF but not set errno in some situations. So this test might be against previous error.

152

In principle ungetc() can return error AKA EOF.

[bdrewery]

Address feedback

• Reset errno before __srefill as it may return EOF without setting it.
• Destaircase
• Handle (malloc failing) EOF error from __ungetc

[bdrewery]

In D31687#714822, @kib wrote:

I believe stdio is not supposed to work (correctly) with non-blocking file descriptors. I do not object against attempts to make part of it work regardless, but I am not sure it is possible without making it all functional.

Thank you. At least for this use I have not had any further trouble. I would not be surprised if more comes up

[kib]

lib/libc/stdio/getdelim.c

152

I suggest to brace the multi-line body of the loop.

[bdrewery]

• add braces around ungetc loop