[LTP] [PATCH v2 1/2] syscalls/fremovexattr: Add fremovexattr() tests
Rafael David Tinoco
rafael.tinoco@linaro.org
Wed Nov 7 16:50:58 CET 2018
>> +static int fd = -1;
>> +static char *got_value;
>
> Why not just static char got_value[XATTR_TEST_VALUE_SIZE]?
Of course =o) !! Will do.
>> +static void cleanup(void)
>> +{
>> + free(got_value);
>> +
>> + close(fd);
> ^
> if (fd > 0)
> SAFE_CLOSE(fd);
Missed this.
>
> would be slightly better here as it would emit
> warning if the close() has failed.
>
>> +}
>> +
>> +static void setup(void)
>> +{
>> + SAFE_TOUCH(FNAME, 0644, NULL);
>> +
>> + fd = SAFE_OPEN(FNAME, O_RDWR, NULL);
> ^
> open() is a variable argument
> function and the third argument is
> an integer but only in a case that
> we passed O_CREAT in the flags, if
> we are not creating the file
> it should be omitted
Sure! Auto mode, sorry.
>> +++ b/testcases/kernel/syscalls/fremovexattr/fremovexattr02.c
>> ...
>> + SAFE_TOUCH(FNAME, 0644, NULL);
>> + fd = SAFE_OPEN(FNAME, O_RDWR, NULL);
>
> Here as well no need for the SAFE_TOUCH().
Same.
>> +static struct tst_test test = {
>> + .setup = setup,
>> + .test = verify_fremovexattr,
>> + .cleanup = cleanup,
>> + .tcnt = ARRAY_SIZE(tc),
>> + .mntpoint = MNTPOINT,
>> + .mount_device = 1,
>> + .all_filesystems = 1,
>> + .needs_tmpdir = 1,
>> + .needs_root = 1,
>> +};
>
> I'm wondering if we need the all_filesystem here, I guess that the
> ENODATA test will reach down to the filesystem driver, so it's probably
> relevant here.
Yep.. that is what I though.
Tks a lot Cyril, will send a v3.
o/
More information about the ltp
mailing list