On Tue, 03 Mar 2009 11:03:58 +0100, Francesco RUNDO <[email protected]> wrote:
> > > Jiří Paleček wrote: > >> On Tue, 03 Mar 2009 10:23:53 +0100, Subrata Modak >> <[email protected]> wrote: >> >>> >>> On Mon, 2009-03-02 at 16:50 +0100, Francesco RUNDO wrote: >>> >>>> I run inotify02 test on SH based arch running LTP-2009 release. I'd >>>> like >>>> to submit a change on the code of inotify02.c file. I propose to >>>> replace >>> >> >> the following code: >> >>>> >>>> while (i < len) { >>>> struct inotify_event *event; >>>> event = (struct inotify_event *) &event_buf[i]; >>>> if (test_num >= TST_TOTAL){ >>>> ................................ >>>> >>>> with: >>>> >>>> while ((i < len)&&(test_num < TST_TOTAL)) { >>>> struct inotify_event *event; >>>> event = (struct inotify_event *) &event_buf[i]; >>>> if ((event_set[test_num].mask == event->mask) && >>>> (! strncmp( event_set[test_num].name, >>>> event->name, event->len))) { >>>> ............................................... >>>> >>>> The detailed change is reported in the attached patch. >>>> >>>> In that way, we avoid to run the code within the above while loop, if >>>> the total amount of the notified events has been reached. >>> >> >> Yes, but why? The original code reported failure in this case, and >> your patch makes this into a PASS. Although it was made in a kludgy >> way, and probably not playing nicely with Tst_count vs. TST_TOTAL, do >> you think it was incorrect, so that you needed to change the behaviour? >> >> Regards >> Jiri Palecek > > Yes, because the test parses, at the end, a sort of "unnecessary" event > reporting, as you rightly say, a failure. Which is the reason to check > it ? 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 >>>> 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 >>> >> >> >> > -- Using Opera's revolutionary e-mail client: http://www.opera.com/mail/ ------------------------------------------------------------------------------ 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
