Hi Todd, hi Andrey,
Todd C. Miller wrote on Wed, Aug 24, 2016 at 10:03:47AM -0600:
> I'm probably not understanding something here
No, i think yours is a great idea, i merely failed to think of it.
> but why can't you just test for __SEOF in fp->_flags
> when __srefill() returns non-zero
> and treat the absence of __SEOF as an error?
See the improved patch below!
Andrey Chernov wrote:
> Yes, it is a bit problematic, but it is the way this function
> designed in BSD 44lite, so I disagree with the patch.
> I.e. preserving partial line is more essential for it than
> returning no line at all (NULL), so tail error on partial line
> is not treated as error.
Do you really think anybody depends on getting a partial line back
right before a low-level read error? Even though it works only
sometimes, not always, almost always returns incomplete data, and
the point where the data will be cut off is almost unpredictable?
> Since this function usually used in the loop, it returns partial
> line on the first pass and returns NULL with error on the second
> pass, so uncatched error on the first pass is not a big deal.
I agree that it's not a big deal for *this* function (only a minibug
as i said). In particular because for byte-oriented streams, it
is an uncommon situation that you can read some bytes but then run
into a read error.
But i'd like to get both getln(3) and getwln(3) fixed, and both of
them in a similar way. And for getwln(3), the situation of being
able to read some wide characters before running into an (encoding)
error is not uncommon at all, and in that case, you noticed yourself
that the *next* read will typically not fail again, so the typical
loop will treat the encoding error as a newline, put the library
into an undefined shift state, and happily go on to read garbage.
Actually, even in getln(3), this can go quite wrong in the form of
a race condition - for example, a program running with input connected
to a terminal using SIG_IGN on SIGTTIN. Imagine the first read
fails with EIO, then the program comes back to the foreground, and
by the time of the next read, that read succeeds again. So some
data is lost, but the temporary error looks exactly like a newline
(unless somebody checks ferror(3) after a successful read, which
isn't very reasonable to do).
So i think fixing fgetln(3) as proposed is worthwhile, both for its
own sake and because it makes it possible to fix the similar, but
more severe problem in fgetwln(3) properly and in a consistent way.
Yours,
Ingo
Index: fgetln.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/fgetln.c,v
retrieving revision 1.14
diff -u -p -r1.14 fgetln.c
--- fgetln.c 31 Aug 2015 02:53:57 -0000 1.14
+++ fgetln.c 24 Aug 2016 17:16:07 -0000
@@ -115,8 +115,12 @@ fgetln(FILE *fp, size_t *lenp)
(void)memcpy((void *)(fp->_lb._base + off), (void *)fp->_p,
len - off);
off = len;
- if (__srefill(fp))
- break; /* EOF or error: return partial line */
+ if (__srefill(fp)) {
+ if (fp->_flags & __SEOF)
+ break;
+ else
+ goto error;
+ }
if ((p = memchr((void *)fp->_p, '\n', fp->_r)) == NULL)
continue;
