> AFAIK, to make sure that the kernel generated those and only those > events, that were predetermined by the test run. Otherwise, we'd have > an inconsistency in the checking procedure. > > Does it happen somewhere, that the kernel legitimately generates > extra events (at the end)? > >> I think the check has to be related only to the notified events (the >> amount of that events should be TST_TOTAL). Moreover, the first run >> of the while loop was made with tst_count=0 so that the while loop >> may reach TST_TOTAL+1 cycles. I think it is useless as the latest >> run is used to check unnecessary event. > > > I don't say I can't imagine a cleaner implementation of checking the > length of the event sequence, but do you really think the test > should't check for it? Why haven't you deleted similar code for > checking an event sequence that is too short? > > Regards > Jiri Palecek
Ok, just to better describe my opinion (I hope without misunderstanding ;-) In the while loop we're analyzing, you rightly process the notified events with different checks.Ok, it is right. My suggest is to avoid to use "also" the check (with TFAIL) related to TST_COUNT as it will be of course executed. In fact, the events to be notified are 9 (the value of TST_COUNT) but if we start with tst_count=0, at the end (tst_count=9) you got a "tst_resm(TFAIL, "get unnecessary event:......." even though "all the notified events" have been rightly processed. I'm not sure about that. Now, my suggest (with related patch) is to avoid to do it by adding the check of TST_COUNT at while level (with "less" condition instead of "less or equal" so that you check only TST_COUNT events). That's it. Anyway, if you wnat to keep this check because you want to check if, after the defined events queue, no further "useful" events will be notified, I think LTP shouldn't retrieve "tst_resm(TFAIL...." but "tst_resm(TPASS...." as expected. Thanks for your comments. Regards -- FR > > >>>>> plain text document attachment >>>>> (ltp-full-20090131-fix-inotify02.patch) >>>>> This patch avoid to run the istructions within the while loop if >>>>> no more events have been collected. >>>>> Signed-off-by: Francesco Rundo <[email protected]> >>>>> --- >>>>> ltp-full-20090131/testcases/kernel/syscalls/inotify/inotify02.c.origin >>>>> >>>>> 2009-03-02 09:16:53.110000000 +0100 >>>>> +++ >>>>> ltp-full-20090131/testcases/kernel/syscalls/inotify/inotify02.c >>>>> 2009-03-02 09:37:04.470002000 +0100 >>>>> @@ -54,7 +54,7 @@ >>>>> #include "test.h" >>>>> #include "usctest.h" >>>>> >>>>> -#if defined(__NR_inotify_init) && defined(HAVE_SYS_INOTIFY) >>>>> +#if defined(__NR_inotify_init) && defined(HAVE_SYS_INOTIFY_H) >>>> >>>> >>>> >>>> Jiri Palecek <[email protected]> already made these changes last >>>> month, Mon Feb 23 07:18:46 2009 UTC. >>>> >>>>> #include <sys/inotify.h> >>>>> >>>>> #ifndef IN_MOVE_SELF >>>>> >>>> >>>> You probably need only the following: >>>> >>>> Signed-Off-By: Francesco RUNDO <[email protected]>, >>>> Signed-Off-By: Subrata Modak <[email protected]>, >>>> -- --- >>>> ltp-full-20090228/testcases/kernel/syscalls/inotify/inotify02.c.orig >>>> 2009-03-03 14:40:23.000000000 +0530 >>>> +++ >>>> ltp-full-20090228/testcases/kernel/syscalls/inotify/inotify02.c >>>> 2009-03-03 14:36:37.000000000 +0530 >>>> @@ -246,25 +246,17 @@ int main(int ac, char **av){ >>>> } >>>> - while (i < len) { >>>> + while ((i < len)&&(test_num < TST_TOTAL)) { >>>> struct inotify_event *event; >>>> event = (struct inotify_event *) &event_buf[i]; >>>> - if (test_num >= TST_TOTAL){ >>>> - tst_resm(TFAIL, "get unnecessary event: " >>>> - "wd=%d mask=%x cookie=%u len=%u" >>>> - "name=\"%s\"",event->wd, event->mask, >>>> - event->cookie, event->len, >>>> - event->name); >>>> - >>>> - } else if ((event_set[test_num].mask == >>>> event->mask) && >>>> - (! strncmp( event_set[test_num].name, >>>> - event->name, event->len))) { >>>> + if ((event_set[test_num].mask == event->mask) && >>>> + (! strncmp( event_set[test_num].name, >>>> + event->name, event->len))) { >>>> tst_resm(TPASS, "get event: wd=%d mask=%x" >>>> - " cookie=%u len=%u name=\"%s\"", >>>> - event->wd, event->mask, >>>> - event->cookie, event->len, >>>> - event->name); >>>> - >>>> + " cookie=%u len=%u name=\"%s\"", >>>> + event->wd, event->mask, >>>> + event->cookie, event->len, >>>> + event->name); >>>> } else { >>>> tst_resm(TFAIL, "get event: wd=%d mask=%x " >>>> "(expected %x) cookie=%u len=%u " >>>> >>>> @@ -375,7 +368,7 @@ >>>> >>>>> tst_resm(TCONF, "Inotify syscall can be found at kernel >>>>> 2.6.13 or higher."); >>>>> return 0; >>>>> #endif >>>>> -#ifndef HAVE_SYS_INOTIFY >>>>> +#ifndef HAVE_SYS_INOTIFY_H >>>> >>>> >>>> >>>> Same here. Changes are already in CVS. >>>> >>>> Regards-- >>>> Subrata >>>> >>>>> tst_resm(TBROK, "can't find header sys/inotify.h"); >>>>> return 1; >>>>> #endif >>>> >>>> >>> >>> >>> >> > > > ------------------------------------------------------------------------------ Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H _______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
