Hi!
> >> if (ret) {
> >> - printf(TNAME " Error at lio_listio() %d: %s\n", errno,
> >> strerror(errno));
> >> + printf (TNAME " Error at lio_listio() %d: %s\n",
> >> + errno, strerror(errno));
> >
> > According to lkml coding style (that we are trying to use here), space
> > should be only between C keywords and (). Not betweeen library calls
> > like printf and so.
>
> This is true for LTP code, but this is third party code. The
> documentation claims to conform to the LKML coding style though (even
> though I know it isn't true).
Wasn't the policy "fix everything that you touch" ?
> >> for (i=0; i<NUM_AIOCBS; i++)
> >> free (aiocbs[i]);
> >> free (bufs);
> >> @@ -165,9 +147,10 @@ main ()
> >> }
> >>
> >> /* Check selected request has not completed yet */
> >> - if (received_selected) {
> >> - printf (TNAME " Error : AIOCB %d already completed before
> >> suspend\n",
> >> - WAIT_FOR_AIOCB);
> >> + err = aio_error(aiocbs[WAIT_FOR_AIOCB]);
> >> + if (err != EINPROGRESS) {
> >> + printf (TNAME " Error : AIOCB %d should not have completed "
> >> + "before suspend\n", WAIT_FOR_AIOCB);
> >> for (i=0; i<NUM_AIOCBS; i++)
> >> free (aiocbs[i]);
> >> free (bufs);
> > ....
> >> exit(PTS_FAIL);
> >>}
> >
> > Another problem lies here. The test relies that aio operation in not
> > finished when we got here. And as some linux kernels fallbacks aio as
> > synchronous IO, when we got there, it's allready finished. So I think we
> > should change this PTS_FAIL to PTS_UNRESOLVED.
> >
> > Garrett what you think?
>
> If an operating system doesn't fully implement a set of requirements,
> it shouldn't claim to do so. If it does then that's a bug in the
> implementation.
I usually agree, but here, it could fail just because testing machine is
too fast or because whatever else happen when scheduling processes/AIO
writes. This is IMHO too fragile to be marked as FAIL.
> >> @@ -178,11 +161,9 @@ main ()
> >>
> >> /* Suspend on selected request */
> >> ret = aio_suspend((const struct aiocb **)plist, 2, NULL);
> >
> > Please remove useless cast to (const struct aiocb**)
>
> Depends on whether or not this is correct. According to GCC it matters:
>
> ../../../conformance/interfaces/aio_suspend/1-1.c:180: warning:
> passing argument 1 of 'aio_suspend' from incompatible pointer type
>
> This should probably have the restrict c99 keyword attached to it though.
Out of curiosity, what OS/gcc version does this? I have no warnings with
all the casts removed from 1-1.c.
> >> -
> >> - /* Check selected request has completed */
> >> - if (!received_selected) {
> >> - printf (TNAME " Error : AIOCB %d should have completed after
> >> suspend\n",
> >> - WAIT_FOR_AIOCB);
> >> + if (ret) {
> >> + printf (TNAME " Error at aio_suspend() %d: %s\n",
> >> + errno, strerror (errno));
> >> for (i=0; i<NUM_AIOCBS; i++)
> >> free (aiocbs[i]);
> >> free (bufs);
> >> @@ -191,9 +172,11 @@ main ()
> >> exit (PTS_FAIL);
> >> }
> >
> > Here again with the printf.
> >
> >> -
> >> - if (ret) {
> >> - printf (TNAME " Error at aio_suspend() %d: %s\n", errno,
> >> strerror (errno));
> >> + /* Check selected request has completed */
> >> + err = aio_error(aiocbs[WAIT_FOR_AIOCB]);
> >> + if (err) {
> >> + printf (TNAME " Error : AIOCB %d should have completed after
> >> "
> >> + "suspend\n", WAIT_FOR_AIOCB);
> >> for (i=0; i<NUM_AIOCBS; i++)
> >> free (aiocbs[i]);
> >> free (bufs);
> >
> > And here.
>
> I've cleaned up the style in this test to be more logical.
> Unfortunately the bull.net contributed tests weren't cleaned up on
> import to posix_testsuite, just like some of the syscall testcases in
> LTP proper.
That's why we are here, to fix the mess ;).
--
Cyril Hrubis
[email protected]
------------------------------------------------------------------------------
Increase Visibility of Your 3D Game App & Earn a Chance To Win $500!
Tap into the largest installed PC base & get more eyes on your game by
optimizing for Intel(R) Graphics Technology. Get started today with the
Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
http://p.sf.net/sfu/intelisp-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
