Garrett Cooper wrote:
> On Thu, Nov 18, 2010 at 10:29 AM, Cyril Hrubis <[email protected]> wrote:
>> Hi!
>>> aio_suspend may be interrupted by IO request's completion signal, so we
>>> should use aio_error instead signal hander to check whether this request
>>> has complete.
>>>
>>> Signed-off-by: Bian Naimeng <[email protected]>
>>>
>>> ---
>>> .../conformance/interfaces/aio_suspend/1-1.c | 51
>>> +++++++-------------
>>> 1 files changed, 17 insertions(+), 34 deletions(-)
>>>
>>> diff --git
>>> a/testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c
>>> b/testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c
>>> index 7d33e62..8c85ca5 100644
>>> ---
>>> a/testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c
>>> +++
>>> b/testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c
>>> @@ -44,19 +44,11 @@
>>> #define BUF_SIZE 1024*1024
>>> #define WAIT_FOR_AIOCB 6
>>>
>>> -int received_selected = 0;
>>> int received_all = 0;
>>>
>>> void
>>> sigrt1_handler(int signum, siginfo_t *info, void *context)
>>> {
>>> - if (info->si_value.sival_int == WAIT_FOR_AIOCB)
>>> - received_selected = 1;
>>> -}
>>> -
... snip ...
>>> /* Setup suspend list */
>>> plist[0] = NULL;
>>> plist[1] = aiocbs[WAIT_FOR_AIOCB];
>>> @@ -155,7 +136,8 @@ main ()
>>> ret = lio_listio(LIO_NOWAIT, aiocbs, NUM_AIOCBS, &event);
>>>
>>> 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).
>
>>> 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.
>
>>> @@ -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.
>
>>> -
>>> - /* 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.
En..., this test is still fail, what's your opinion about this patch?
Thanks
Bian
>
> Thanks,
> -Garrett
>
------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3.
Spend less time writing and rewriting code and more time creating great
experiences on the web. Be a part of the beta today
http://p.sf.net/sfu/msIE9-sfdev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
