On 14 March 2013 17:22, <[email protected]> wrote:
> Hi!
>> diff --git a/testcases/kernel/syscalls/getdents/Makefile
>> b/testcases/kernel/syscalls/getdents/Makefile
>> index df7b63f..8334aa7 100644
>> --- a/testcases/kernel/syscalls/getdents/Makefile
>> +++ b/testcases/kernel/syscalls/getdents/Makefile
>> @@ -21,6 +21,8 @@ top_srcdir ?= ../../../..
>> include $(top_srcdir)/include/mk/testcases.mk
>> include $(abs_srcdir)/../utils/newer_64.mk
>>
>> +CFLAGS += -D_LARGEFILE64_SOURCE
>
> What exactly is this needed for? The getent64 function and
> linux_dirent64 are both declared in the getdents.h.
It's needed for getting the struct dirent64 definition. See
/usr/include/bits/dirent.h
>> + ptrs.buf = buf;
>> + ret = ltp_syscall(__NR_getdents64, fd, buf, count);
>> if (ret < 0)
>> return ret;
>
> Well my choice would be casting the buffer to the dirp pointer rather
> than using the union but looking at the original code, this has been
> there before...
Yep
>
>> diff --git a/testcases/kernel/syscalls/getdents/getdents04.c
>> b/testcases/kernel/syscalls/getdents/getdents04.c
>> index 89479a4..7ba8a3d 100644
>> --- a/testcases/kernel/syscalls/getdents/getdents04.c
>> +++ b/testcases/kernel/syscalls/getdents/getdents04.c
>> @@ -43,10 +43,12 @@
>> * -e : Turn on errno logging.
>> * -i n : Execute test n times.
>> * -I x : Execute test for x seconds.
>> + * -l : Test the getdents64 system call.
>> * -P x : Pause for x seconds between iterations.
>> * -t : Turn on syscall timing.
>> *
>> * HISTORY
>> + * 03/2013 - Added -l option by Markos Chandras
>> * 03/2001 - Written by Wayne Boyer
>> *
>> * RESTRICTIONS
>> @@ -73,6 +75,17 @@ int TST_TOTAL = 1;
>>
>> int exp_enos[] = { ENOTDIR, 0 }; /* 0 terminated list of expected
>> errnos */
>>
>> +int longsyscall;
>> +
>> +option_t Options[] = {
>> + {"l", &longsyscall, NULL}, /* -l long option. Tests getdents64
>> */
>> + {NULL, NULL, NULL}
>> +};
>> +
>> +void help() {
>> + printf(" -l Test the getdents64 system call\n");
>> +}
>
> Please use LKML coding style, i.e. add void to the function prototype
> and move the opening bracket on the new line (you can you chekpatch.pl
> bundled with the linux kernel sources to check for style violations).
Ok I just copied what other tests do so I thought this is the desired
coding style. I will fix it.
>> + if ((dirp64 = malloc(sizeof(struct dirent64))) == NULL)
>> + tst_brkm(TBROK, cleanup, "malloc failed");
>> + count = (int)sizeof(struct dirent64);
>
> Does really assigment of size_t (return from sizeof) into the int
> (count) generate warning?
I haven't checked, but this was there before as well
>>
>> - free(dirp);
>> + if (longsyscall)
>> + free(dirp64);
>> + else
>> + free(dirp);
>
> Hmm allocatin and freeing the structure in each loop iteration seems
> pointless, what about defining it on the stack or allocating it once
> before the main test loop and not bother freeing it. That would simplify
> the code.
I guess we could do that too. I will fix it. Thanks for the review
--
Regards,
Markos Chandras
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
