Message ID | CALoOobNoDoSCD2bTsgY-CkHtSmwMcXk0fmvK5Vbgc9XpeGWi-w@mail.gmail.com |
---|---|
State | New |
Headers | show |
Hi, Thanks for the patch, it straightforward enough. Only a comment below. On 02-08-2015 17:16, Paul Pluzhnikov wrote: > Greetings, > > Attached trivial patch fixes BZ 18756 and adds a test case for it. > Since this is new breakage in 2.22, I say it should go in despite the > hard freeze. > > Thanks, > > 2015-08-02 Paul Pluzhnikov <ppluzhnikov@google.com> > > [BZ #18756] > * libio/fmemopen.c (__fmemopen): Check for 0 len. > * libio/test-fmemopen.c (do_bz18756): New test. > > -- Paul Pluzhnikov > > > bz18756-20150802.txt > > > diff --git a/libio/fmemopen.c b/libio/fmemopen.c > index 3ab3e8d..c58f376 100644 > --- a/libio/fmemopen.c > +++ b/libio/fmemopen.c > @@ -150,6 +150,12 @@ __fmemopen (void *buf, size_t len, const char *mode) > cookie_io_functions_t iof; > fmemopen_cookie_t *c; > > + if (__glibc_unlikely (len == 0)) > + { > + __set_errno (EINVAL); > + return NULL; > + } > + > c = (fmemopen_cookie_t *) calloc (sizeof (fmemopen_cookie_t), 1); > if (c == NULL) > return NULL; > diff --git a/libio/test-fmemopen.c b/libio/test-fmemopen.c > index 63ca89f..81371fa 100644 > --- a/libio/test-fmemopen.c > +++ b/libio/test-fmemopen.c > @@ -24,6 +24,27 @@ static char buffer[] = "foobar"; > #include <errno.h> > > static int > +do_bz18756 (void) > +{ > + int ch; > + int ret = 0; > + FILE *stream; > + > + errno = 0; > + stream = fmemopen (&ch, 0, "w"); > + if (stream != NULL || errno != EINVAL) > + { > + printf ("fmemopen zero-sized buffer: stream = %p, %m\n", stream); > + ret = 1; > + } I would use the 'FAIL:' message pattern to follow recent GLIBC testcases here. > + > + if (stream != NULL) > + fclose (stream); > + > + return ret; > +} > + > +static int > do_test (void) > { > int ch; > @@ -44,7 +65,7 @@ do_test (void) > > fclose (stream); > > - return ret; > + return ret + do_bz18756 (); > } > > #define TEST_FUNCTION do_test ()
On Mon, Aug 03 2015, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > Hi, > > Thanks for the patch, it straightforward enough. Only a comment below. > But why? What happened to https://sourceware.org/bugzilla/show_bug.cgi?id=11216 ? What I_WANT_SANE_FMEMOPEN_SEMANTICS flag should I set to avoid having to special-case the empty string in fmemopen(s, strlen(s), "r")? Why should a memory buffer which happens to be of length 0 be treated any differently than fopen() of an empty file [or /dev/full in the case of writing]? Or put another way, in what scenario is it useful for fmemopen() to return NULL/errno==EINVAL when len==0? "The standard says/said so" is not a valid reason: that would just imply that the standard is broken. Which they sort-of seem to have acknowledged. Please, let's keep trying to steer the supertanker in the right direction instead of mindlessly following it along its misdirected course. Rasmus
On Mon, Aug 3, 2015 at 6:35 AM, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote: > But why? What happened to > https://sourceware.org/bugzilla/show_bug.cgi?id=11216 ? I didn't know about that one. Thanks, patch withdrawn.
Thanks for remind about this one (something was lingering in my head saying this has already been addressed). On 03-08-2015 10:35, Rasmus Villemoes wrote: > On Mon, Aug 03 2015, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > >> Hi, >> >> Thanks for the patch, it straightforward enough. Only a comment below. >> > > But why? What happened to > https://sourceware.org/bugzilla/show_bug.cgi?id=11216 ? > > What I_WANT_SANE_FMEMOPEN_SEMANTICS flag should I set to avoid having to > special-case the empty string in fmemopen(s, strlen(s), "r")? > > Why should a memory buffer which happens to be of length 0 be treated > any differently than fopen() of an empty file [or /dev/full in the case > of writing]? Or put another way, in what scenario is it useful for > fmemopen() to return NULL/errno==EINVAL when len==0? > > > "The standard says/said so" is not a valid reason: that would just imply > that the standard is broken. Which they sort-of seem to have > acknowledged. Please, let's keep trying to steer the supertanker in the > right direction instead of mindlessly following it along its misdirected > course. > > Rasmus >
diff --git a/libio/fmemopen.c b/libio/fmemopen.c index 3ab3e8d..c58f376 100644 --- a/libio/fmemopen.c +++ b/libio/fmemopen.c @@ -150,6 +150,12 @@ __fmemopen (void *buf, size_t len, const char *mode) cookie_io_functions_t iof; fmemopen_cookie_t *c; + if (__glibc_unlikely (len == 0)) + { + __set_errno (EINVAL); + return NULL; + } + c = (fmemopen_cookie_t *) calloc (sizeof (fmemopen_cookie_t), 1); if (c == NULL) return NULL; diff --git a/libio/test-fmemopen.c b/libio/test-fmemopen.c index 63ca89f..81371fa 100644 --- a/libio/test-fmemopen.c +++ b/libio/test-fmemopen.c @@ -24,6 +24,27 @@ static char buffer[] = "foobar"; #include <errno.h> static int +do_bz18756 (void) +{ + int ch; + int ret = 0; + FILE *stream; + + errno = 0; + stream = fmemopen (&ch, 0, "w"); + if (stream != NULL || errno != EINVAL) + { + printf ("fmemopen zero-sized buffer: stream = %p, %m\n", stream); + ret = 1; + } + + if (stream != NULL) + fclose (stream); + + return ret; +} + +static int do_test (void) { int ch; @@ -44,7 +65,7 @@ do_test (void) fclose (stream); - return ret; + return ret + do_bz18756 (); } #define TEST_FUNCTION do_test ()