Re: [PATCH 2/3] libio: Update tst-fmemopen2.c

[Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>]

On 23-06-2014 15:39, Rasmus Villemoes wrote:
> Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes:
>
>> This patch updates tst-fmemopen2 to check for fmemopen with NULL buffer
>> inputs and also refactor the code a bit.
>>
>> --
>>
>> 	* stdio-common/tst-fmemopen2.c (do_test): Add test for NULL and zero
>> 	length buffers.
>>
>> ---
>>
>> diff --git a/stdio-common/tst-fmemopen2.c b/stdio-common/tst-fmemopen2.c
>> index c2a4baa..652c412 100644
>> --- a/stdio-common/tst-fmemopen2.c
>> +++ b/stdio-common/tst-fmemopen2.c
> [snip]
>
>> +}
>> +
>> +static int
>> +do_test_length_zero (void)
>> +{
>> +  int result = 0;
>> +  FILE *fp;
>> +  char buf[] = "";
>> +  const size_t nbuf = 0;
>> +  int r;
>> +
>> +  fp = fmemopen (buf, nbuf, "r");
>> +  if (fp == NULL)
>> +    {
>> +      printf ("%s: fmemopen failed\n", __FUNCTION__);
>> +      return 1;
>> +    }
>> +
>> +  /* Reading any data on a zero-length buffer should return EOF  */
> Missing full stop.
>
>> +  if ((r = fgetc (fp)) != EOF)
>> +    {
>> +      printf ("%s: fgetc on a zero-lenght returned: %d\n",
> s/lenght/length/
>
>> +	      __FUNCTION__, r);
>> +      result = 1;
>> +    }
>> +  off_t o = ftello (fp);
>> +  if (o != 0)
>> +    {
>> +      printf ("%s: first ftello returned %ld, expected 0\n",
>> +	      __FUNCTION__, o);
>> +      result = 1;
>> +    }
>> +  fclose (fp);
>> +
>> +  /* Writing any data shall start at current position and shall not pass
>> +     current buffer size beyond the size in fmemopen call.  */
>> +  fp = fmemopen (buf, nbuf, "w");
>> +  if (fp == NULL)
>> +    {
>> +      printf ("%s: second fmemopen failed\n", __FUNCTION__);
>> +      return 1;
>> +    }
>> +
>> +  static const char str[] = "hello world";
>> +  /* Because of buffering, fputs call itself don't fail, however the final
>> +     buffer should be not changed because of lenght 0 passed in fmemopen
> s/lenght/length/
>
>> +     call.  */
>> +  fputs (str, fp);
> I think it would be useful to do an fflush(fp) and check that the error
> flag is set, and perhaps also to check that errno is set to ENOSPC.
>
>> +  fclose (fp);
>> +
>> +  if (strcmp (buf, "") != 0)
>> +    {
>> +      printf ("%s: strcmp (%s, "") failed\n", __FUNCTION__, buf);
>> +      return 1;
>> +    }
>> +
> Maybe it would be a bit more robust if buf was declared with a somewhat
> larger size than the 1 implied from its initialization with "", so that
> if the write test fails, adjacent stack variables are not necessarily
> overwritten.
>
>
> Rasmus
>
> PS: Thanks for allowing len==0 :-)
>
Thanks, I have added all the comments in my patch set.  I will send them again shortly.