On Tue, 18 Nov 2008 08:31:55 +0100, Subrata Modak <[EMAIL PROTECTED]> 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
<[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.

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/

Attachment: 0001-Compile-the-accept4-test-on-kernels-that-lack-it.patch
Description: Binary data

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