From: Jiří P. <jpa...@we...> - 2008-11-18 11:13:32
|
On Tue, 18 Nov 2008 08:31:55 +0100, Subrata Modak <su...@li...> wrote: > > On Tue, 2008-11-18 at 01:15 +0100, Jiří Paleček wrote: >> Hello, >> >> On Mon, 17 Nov 2008 12:19:13 +0100, Subrata Modak >> <su...@li...> wrote: >> >> > >> > On Fri, 2008-11-14 at 12:40 -0500, Michael Kerrisk wrote: >> >> > Here's the latest version, for review-n-test enjoyment: >> >> >> >> I made a few small tweaks to the changelog: wording fixes, and para >> >> break fixes; other than that, changed the text to say that I rewrote >> >> (rather than updated) the test program. Please substitute it with >> the >> >> following. >> >> >> > I just did a quick patch for your test to run on LTP. If you have no >> > issue(s), can we add this to LTP ? >> > >> > LTP-list, >> > >> > Can you also enrich this patch ? >> >> I have some nitpicks, in decreasing severity: >> >> First, the syscall, I believe, is not targeted at i386 and x86-64 only. >> Therefore, it is not wise to have these explicitly mentioned in the >> code. >> Also, it would be better not to "#error" if the arch isn't one of those >> fortunate, because ltp should build on others too. This should be fixed >> by >> patch 1. >> >> Disclaimer: This patch should make it compile (and fail at runtime with >> TCONF) on all kernels that don't have the syscall, and actually run the >> test on all kernels that do, depending on kernel headers version. >> However, >> I didn't test this (especially the selection of the syscall), so it >> needs >> to be checked. >> >> Second, if any of the syscalls vital for the test fails, it's preferable >> to output the error message too, and call tst_brk() for cleanup (patch >> 2). >> >> Third, there it would probably be better to use TFAIL/TPASS for >> recording >> success and failure instead of manual boolean flags (patch 3). >> >> Last, I think a successful test should print as little as possible and >> multiline messages like "calling syscall..." are really not that useful. >> Patch 4 disables them. > Thanks Jiri for taking out time to investigate and coming out with a > clean implementation for this new test case. I have the following > points: > > 1) Patch no. 1 will fail to apply due to absence of > ltp/testcases/kernel/include/stub-list in the original source code. I > think you wanted to modify testcases/kernel/include/stub-list.orig. Even That's because vapier deleted that file 5 days ago without telling anyone :( More on that in a separate mail. > without this file, the patch will fail to compile with the following > error: > > cc -Wall -I../../include -g -Wall -I../../../../include -Wall > accept4_01.c -L../../../../lib -lltp -o accept4_01 > accept4_01.c: In function ‘accept4’: > accept4_01.c:176: error: ‘__NR_accept4’ undeclared (first use in this > function) > accept4_01.c:176: error: (Each undeclared identifier is reported only > once > accept4_01.c:176: error: for each function it appears in.) > make[4]: *** [accept4_01] Error 1 The corrected (though untested) patch is attached. > 2) Other Patches apply with lots of HUNKS. Please take a clean diff. Well, they apply cleanly if you apply them in sequence, and they apply fine individually (except for 4, which needs 3). I think I cannot do much more when the patches all deal with the same file. > 3) Being too ambitious, i would also request you to kindly introduce > other features that we normally have for a system call test in LTP like: > i) No. of loops to run, > ii) Time, etc, > albeit in your spare time. I don't promise anything - the biggest problem with implementing these features is that I don't know what are they for and how do they work, so I would probably just copy&paste them from a different test. Regards Jiri Palecek -- Using Opera's revolutionary e-mail client: http://www.opera.com/mail/ |