This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ 15362] Fix fwrite() reading beyond end of buffer in error path
- From: Siddhesh Poyarekar <siddhesh at redhat dot com>
- To: Eric Biggers <ebiggers3 at gmail dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 22 Apr 2013 09:57:17 +0530
- Subject: Re: [PATCH][BZ 15362] Fix fwrite() reading beyond end of buffer in error path
- References: <20130417043114 dot GA4239 at zzz dot kirk dot macalester dot edu> <20130417124625 dot GE10160 at spoyarek dot pnq dot redhat dot com> <20130420181218 dot GA6578 at zzz dot student-wireless01 dot macalester dot edu>
On Sat, Apr 20, 2013 at 01:12:18PM -0500, Eric Biggers wrote:
> I must say that running the test suite was not particularly easy for a newcomer.
> There were over a hundred test failures for no apparent reason regardless of
> whether my patch was applied. Finally, based on the wiki (`make check' really
> should warn you about this itself...), I configured with '--prefix=/usr', which
Yes this is a known problem. I'm sure there's a bug report for it
somewhere.
> fixed most of the failing tests. However, some tests still failed even without
> my patch applied (using the master branch at commit
> e913141d5f4d4eed4b65f55b0077aeb1c8234e25):
>
> make[2]: *** [/home/e/src/glibc/build-master2/math/test-ldouble.out] Error 1
> make[2]: *** [/home/e/src/glibc/build-master2/math/test-ildoubl.out] Error 1
> make[1]: *** [math/tests] Error 2
Do you have an amd processor? I think I had seen math test failures
on my amd fx4100 a while ago and had made a mental note to look into
it. Like all mental notes, this one also got swapped out I guess. In
any case, you can ignore these for your change.
> make[2]: [/home/e/src/glibc/build-master2/posix/annexc.out] Error 1 (ignored)
> make[2]: *** [/home/e/src/glibc/build-master2/rt/tst-cpuclock2.out] Error 1
The rt tests are unstable, so this one is known to fail sporadically.
> If there are tests that are expected to fail in some situations, is there any
> way that the test suite itself could warn of that?
>
> I was also surprised that there seemed to be no place where a clear summary of
> the test suite results gets logged, other than by redirecting the standard error
> of 'make check -k' and grepping for the word "Error" to get past all the
> compiler warnings that also get sent to stderr. Is there a better way to run
> the tests?
Nope, that's the best I know. There's plenty of scope for
improvements to the testsuite.
> The fixed patch follows, although note there are still other comments in the
> same source code file that do not follow this style guideline.
They should be fixed too, but not in this patch.
> diff --git a/ChangeLog b/ChangeLog
> index 90d6d47..e978ee2 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,14 @@
> +2013-04-20 Eric Biggers <ebiggers3@gmail.com>
> +
> + [BZ #15362]
> + * libio/fileops.c: Revert problematic fixes for [BZ #11741]
> + * libio/iofwrite.c: Likewise.
> + * libio/iofwrite_u.c: Likewise.
> + * libio/iopadn.c: Likewise.
> + * libio/iowpadn.c: Likewise.
> + * stdio-common/vfprintf.c: Fix [BZ #11741] properly by checking whether
> + _IO_padn() returned the full count written.
> +
> 2013-04-16 Roland McGrath <roland@hack.frob.com>
>
The change looks fine to me now. As I had mentioned before, I'll wait
for a review from another maintainer and check it in if there's no
objection.
Thanks,
Siddhesh