grep testsuite: test-fwrite fails with git glibc
Siddhesh Poyarekar
siddhesh@redhat.com
Sun Nov 25 08:53:00 GMT 2012
On Sun, Nov 25, 2012 at 07:17:06AM +0100, Andreas Jaeger wrote:
> With glibc git, I get a test failure in building grep:
>
> grep-2.14/gnulib-tests> ./test-fwrite
> test-fwrite.c:53: assertion failed
> Aborted
>
> This does not seem to occur with glibc 2.16.
> Siddhesh, could you check the grep testsuite with your patch for
> BZ#11741, please?
My patch seems to have exposed what I think is a bug in fwrite
implementation. The current assumption in fwrite is that even if
_IO_sputn returns EOF, the data is in the buffer and only a flush
failed. That assumption is wrong since the flush could have happened
in the middle of writing data, which resulted in an EOF. The
following patch to fwrite should fix this. The glibc testsuite is
still clean with this change. Is this OK for master?
Siddhesh
ChangeLog:
[BZ #11741]
* libio/iofwrite (_IO_fwrite): Return 0 on EOF.
* libio/iofwrite_u.c (fwrite_unlocked): Likewise.
-------------- next part --------------
diff --git a/libio/iofwrite.c b/libio/iofwrite.c
index d4610f7..e93a656 100644
--- a/libio/iofwrite.c
+++ b/libio/iofwrite.c
@@ -43,11 +43,19 @@ _IO_fwrite (buf, size, count, fp)
written = _IO_sputn (fp, (const char *) buf, request);
_IO_release_lock (fp);
/* We have written all of the input in case the return value indicates
- this or EOF is returned. The latter is a special case where we
- simply did not manage to flush the buffer. But the data is in the
- buffer and therefore written as far as fwrite is concerned. */
- if (written == request || written == EOF)
+ this. */
+ if (written == request)
return count;
+ /* It is possible that the data was written out into buffer and we just
+ failed to flush it out. However, this is not necessarily always the
+ case and we cannot really differentiate this with a case when a flush
+ failed and all of the data was not in the buffer. Hence, just return 0
+ (the flush failure should already have set the errno) and let the user
+ decide what to do. A future enhancement could be to find out how much
+ data is in the buffer and return that as a short write instead of just
+ 0. */
+ else if (written == EOF)
+ return 0;
else
return written / size;
}
diff --git a/libio/iofwrite_u.c b/libio/iofwrite_u.c
index a1077ee..bc533dd 100644
--- a/libio/iofwrite_u.c
+++ b/libio/iofwrite_u.c
@@ -45,11 +45,19 @@ fwrite_unlocked (buf, size, count, fp)
{
written = _IO_sputn (fp, (const char *) buf, request);
/* We have written all of the input in case the return value indicates
- this or EOF is returned. The latter is a special case where we
- simply did not manage to flush the buffer. But the data is in the
- buffer and therefore written as far as fwrite is concerned. */
- if (written == request || written == EOF)
+ this. */
+ if (written == request)
return count;
+ /* It is possible that the data was written out into buffer and we just
+ failed to flush it out. However, this is not necessarily always the
+ case and we cannot really differentiate this with a case when a flush
+ failed and all of the data was not in the buffer. Hence, just return 0
+ (the flush failure should already have set the errno) and let the user
+ decide what to do. A future enhancement could be to find out how much
+ data is in the buffer and return that as a short write instead of just
+ 0. */
+ else if (written == EOF)
+ return 0;
}
return written / size;
More information about the Libc-alpha
mailing list