Fix BZ 18756 (fmemopen(..., 0, ...) does not fail)

  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.
  

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
>