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  
> <[EMAIL PROTECTED]> 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.
> 
> Regards
>      Jiri Palecek

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
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

2) Other Patches apply with lots of HUNKS. Please take a clean diff.
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 am expecting a fresh set of patches from you to address the bare
minimum over the patch i sent yesterday.

Regards--
Subrata



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to