Re: [PATCH v2 2/3] Consolidate posix_fallocate{64} implementations

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 05/10/2016 14:02, Siddhesh Poyarekar wrote:
>> +
>> +static void
>> +do_prepare (void)
>> +{
>> +  temp_fd = create_temp_file ("tst-posix_fallocate.", &temp_filename);
>> +  if (temp_fd == -1)
>> +    {
>> +      printf ("cannot create temporary file: %m\n");
>> +      exit (1);
>> +    }
>> +}
>> +
>> +#define FAIL(str) \
>> +  do { printf ("error: %s (line %d)\n", str, __LINE__); return 1; } while (0)
> 
> Move the definition from tst-fallocate-common.c out into test-skeleton.c
> and use that one instead of this.

What about the new definition I sent with fallocate update patch:

#define FAIL(...) \
  ({ \
     printf ("error: line %d: ", __LINE__); \
     printf (__VA_ARGS__); \
     printf ("\n"); \
     return 1; \
  })

I think I can rename to FAIL_RET to explicit state it returns and maybe
add a RET_EXIT with same logic but calling exit.

>> +
>> +static int
>> +do_test_with_offset (off_t offset)
>> +{
>> +  struct stat st;
>> +
>> +  if (posix_fallocate (temp_fd, offset, 768) != 0)
>> +    FAIL ("1st posix_fallocate call failed");
>> +
>> +  if (fstat (temp_fd, &st) != 0)
>> +    FAIL ("2nd fstat failed");
>> +
>> +  if (st.st_size != (offset + 768))
>> +    {
>> +      printf ("file size after first posix_fallocate call is %llu, expected %u\n",
>> +	      (unsigned long long int) st.st_size, 512u + 768u);
>> +      return 1;
> 
> Use the new FAIL macro from tst-fallocate-common.c (or rather, in
> test-skeleton.c after you have moved it).

Ack.

> 
>> +    }
>> +
>> +  if (posix_fallocate (temp_fd, 0, 1024) != 0)
>> +    FAIL ("2nd posix_fallocate call failed");
>> +
>> +  if (fstat (temp_fd, &st) != 0)
>> +    FAIL ("3rd fstat failed");
>> +
>> +  if (st.st_size != (offset) + 768)
>> +    FAIL ("file size changed in second posix_fallocate");
>> +
>> +  offset += 2048;
>> +  if (posix_fallocate (temp_fd, offset, 64) != 0)
>> +    FAIL ("3rd posix_fallocate call failed");
>> +
>> +  if (fstat (temp_fd, &st) != 0)
>> +    FAIL ("4th fstat failed");
>> +
>> +  if (st.st_size != (offset + 64))
>> +    {
>> +      printf ("file size after first posix_fallocate call is %llu, expected %u\n",
>> +	      (unsigned long long int) st.st_size, 2048u + 64u);
> 
> Likewise.

Ack.

> 
>> +      return 1;
>> +    }
>> +
>> +  return 0;
>> +}
>> diff --git a/io/tst-posix_fallocate.c b/io/tst-posix_fallocate.c
>> index 53f0704..59f3fa1 100644
>> --- a/io/tst-posix_fallocate.c
>> +++ b/io/tst-posix_fallocate.c
>> @@ -1,100 +1,33 @@
>> -#include <fcntl.h>
>> -#include <sys/stat.h>
>> +/* Basic posix_fallocate tests.
>> +   Copyright (C) 2016 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>>  
>> -static void do_prepare (void);
>> -#define PREPARE(argc, argv) do_prepare ()
>> -static int do_test (void);
>> -#define TEST_FUNCTION do_test ()
>> -#include <test-skeleton.c>
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>>  
>> -static int fd;
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>>  
>> -static void
>> -do_prepare (void)
>> -{
>> -  fd = create_temp_file ("tst-posix_fallocate.", NULL);
>> -  if (fd == -1)
>> -    {
>> -      printf ("cannot create temporary file: %m\n");
>> -      exit (1);
>> -    }
>> -}
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>>  
>> +#include "tst-posix_fallocate-common.c"
>>  
>>  static int
>>  do_test (void)
>>  {
>> -  struct stat64 st;
>> +  struct stat st;
>>  
>> -  if (fstat64 (fd, &st) != 0)
>> -    {
>> -      puts ("1st fstat failed");
>> -      return 1;
>> -    }
>> +  if (fstat (temp_fd, &st) != 0)
>> +    FAIL ("1st fstat failed");
>>  
>>    if (st.st_size != 0)
>> -    {
>> -      puts ("file not created with size 0");
>> -      return 1;
>> -    }
>> -
>> -  if (posix_fallocate (fd, 512, 768) != 0)
>> -    {
>> -      puts ("1st posix_fallocate call failed");
>> -      return 1;
>> -    }
>> -
>> -  if (fstat64 (fd, &st) != 0)
>> -    {
>> -      puts ("2nd fstat failed");
>> -      return 1;
>> -    }
>> -
>> -  if (st.st_size != 512 + 768)
>> -    {
>> -      printf ("file size after first posix_fallocate call is %llu, expected %u\n",
>> -	      (unsigned long long int) st.st_size, 512u + 768u);
>> -      return 1;
>> -    }
>> -
>> -  if (posix_fallocate (fd, 0, 1024) != 0)
>> -    {
>> -      puts ("2nd posix_fallocate call failed");
>> -      return 1;
>> -    }
>> -
>> -  if (fstat64 (fd, &st) != 0)
>> -    {
>> -      puts ("3rd fstat failed");
>> -      return 1;
>> -    }
>> -
>> -  if (st.st_size != 512 + 768)
>> -    {
>> -      puts ("file size changed in second posix_fallocate");
>> -      return 1;
>> -    }
>> -
>> -  if (posix_fallocate (fd, 2048, 64) != 0)
>> -    {
>> -      puts ("3rd posix_fallocate call failed");
>> -      return 1;
>> -    }
>> -
>> -  if (fstat64 (fd, &st) != 0)
>> -    {
>> -      puts ("4th fstat failed");
>> -      return 1;
>> -    }
>> -
>> -  if (st.st_size != 2048 + 64)
>> -    {
>> -      printf ("file size after first posix_fallocate call is %llu, expected %u\n",
>> -	      (unsigned long long int) st.st_size, 2048u + 64u);
>> -      return 1;
>> -    }
>> -
>> -  close (fd);
>> +    FAIL ("file not created with size 0");
>>  
>> -  return 0;
>> +  return do_test_with_offset (512);
>>  }
>> diff --git a/io/tst-posix_fallocate64.c b/io/tst-posix_fallocate64.c
>> new file mode 100644
>> index 0000000..cbb57ff
>> --- /dev/null
>> +++ b/io/tst-posix_fallocate64.c
>> @@ -0,0 +1,44 @@
>> +/* Basic posix_fallocate tests (with _FILE_OFFSET_BITS).
>> +   Copyright (C) 2016 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +#define _FILE_OFFSET_BITS 64
>> +#include "tst-posix_fallocate-common.c"
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  struct stat st;
>> +  int ret;
>> +
>> +  if (fstat (temp_fd, &st) != 0)
>> +    FAIL ("1st fstat failed");
>> +
>> +  if (st.st_size != 0)
>> +    FAIL ("file not created with size 0");
>> +
>> +  ret = do_test_with_offset (512);
>> +  if (ret == -1)
>> +    return -1;
> 
> do_test_with_offset returns 1, not -1.
> 

Ack.

>> +
>> +  off_t base_offset = UINT32_MAX + 512LL;
>> +  ret = do_test_with_offset (base_offset);
>> +  if (ret == -1)
>> +    return 1;
> 
> Likewise.
> 

Ack.