Re: [PATCH v2 1/3] Consolidate fallocate{64} implementations

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

On 05/10/2016 03:53, Siddhesh Poyarekar wrote:
> On Tuesday 04 October 2016 08:37 PM, Adhemerval Zanella wrote:
>>>> +
>>>> +  off_t base_offset = UINT32_MAX + 2048LL;
>>>
>>> This will overflow on 32-bit.
>>
>> Not really since we explicit build the test with _FILE_OFFSET_BITS=64 so
>> off_t will be 64-bit regardless.
> 
> I could see that in tst-fallocate, but not in tst-fallocate64.

Yes, but the large offset that exceeds a 32-bit value is checked only on
tst-fallocate64. The tst-fallocate, which uses default 32-bit off_t (on
architectures that do not have off_t 64-bit as default), only tests
offset less than INT_MAX.

>> diff --git a/sysdeps/unix/sysv/linux/tst-fallocate-common.c b/sysdeps/unix/sysv/linux/tst-fallocate-common.c
>> new file mode 100644
>> index 0000000..76f31ff
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/tst-fallocate-common.c
>> @@ -0,0 +1,98 @@
>> +/* Basic fallocate test (no specific flags is checked).
>> +   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/>.  */
>> +
>> +#include <fcntl.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <unistd.h>
>> +
>> +static void do_prepare (void);
>> +#define PREPARE(argc, argv)     do_prepare ()
>> +static int do_test (void);
>> +#define TEST_FUNCTION           do_test ()
>> +
>> +#define TIMEOUT 20 /* sec.  */
>> +
>> +#define XSTR(s) STR(S)
>> +#define STR(s)  #s
>> +
>> +#include <test-skeleton.c>
>> +
>> +static char *temp_filename;
>> +static int temp_fd;
>> +
>> +void
>> +do_prepare (void)
>> +{
>> +  temp_fd = create_temp_file ("tst-fallocate.", &temp_filename);
>> +  if (temp_fd == -1)
>> +    {
>> +      printf ("cannot create temporary file: %m\n");
>> +      exit (1);
>> +    }
>> +}
>> +
>> +#define FAIL(...) \
>> +  ({ \
>> +     printf ("error: line %d: ", __LINE__); \
>> +     printf (__VA_ARGS__); \
>> +     printf ("\n"); \
>> +     return 1; \
>> +  })
>> +
>> +static int
>> +do_test_with_offset (off_t offset)
>> +{
>> +  int ret;
>> +  struct stat finfo;
>> +#define BLK_SIZE 1024
>> +  char bwrite[BLK_SIZE] = { 0xf0 };
>> +  char bread[BLK_SIZE];
>> +
>> +  /* It tries to fallocate 1024 bytes from 'offset' and then write 1024 bytes.
>> +     After both operation rewind the file descriptor and read 1024 bytes
>> +     and check if both buffer have the same contents.  */
>> +  ret = fallocate (temp_fd, 0, offset, BLK_SIZE);
>> +  if (ret == -1)
>> +    FAIL ("fallocate failed");
>> +
>> +  ret = fstat (temp_fd, &finfo);
>> +  if (ret == -1)
>> +    FAIL ("fstat failed");
>> +
>> +  if (finfo.st_size < (offset + BLK_SIZE))
>> +    FAIL ("size of first fallocate less than expected (%llu)",
>> +	  (long long unsigned int)offset + BLK_SIZE);
>> +
>> +  if (lseek (temp_fd, offset, SEEK_SET) == (off_t) -1)
>> +    FAIL ("fseek (0, SEEK_SET) failed");
>> +
>> +  if (write (temp_fd, bwrite, BLK_SIZE) != BLK_SIZE)
>> +    FAIL ("fail trying to write " XSTR (BLK_SIZE) " bytes");
>> +
>> +  if (lseek (temp_fd, offset, SEEK_SET) == (off_t) -1)
>> +    FAIL ("fseek (0, SEEK_SET) failed");
>> +
>> +  if (read (temp_fd, bread, BLK_SIZE) != BLK_SIZE)
>> +    FAIL ("fail trying to read " XSTR (BLK_SIZE) " bytes");
>> +
>> +  if (memcmp (bwrite, bread, BLK_SIZE) != 0)
>> +    FAIL ("buffer writted different than buffer readed");
> 
> Typo, "written"?

Yes, I will fix it.