Re: [LTP] [PATCH] aio_fsync:4-2: wait until the io is in no INPROGRESS

Re: [LTP] [PATCH] aio_fsync:4-2: wait until the io is in no INPROGRESS

[Cyril Hrubis]

Hi!
> If the aio is in INPROGRESS, the aio_return is undefined, and
> may cause Segment Faul.
> So, wait for it.
> 
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: lidan <li.dan@cn.fujitsu.com>
> ---
>  .../conformance/interfaces/aio_fsync/4-2.c         |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-2.c b/testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-2.c
> index bf76109..2be8d4b 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-2.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-2.c
> @@ -68,6 +68,7 @@ int main()
>  		exit(PTS_FAIL);
>  	}
>  
> +	while(aio_error(&aiocb_fsync) == EINPROGRESS);
>  	errno = 0;
>  	aio_return(&aiocb_fsync);
>  	if (errno != 0)
> @@ -78,4 +79,4 @@ int main()
>  	close(fd);
>  	printf ("Test PASSED\n");
>  	return PTS_PASS;
> -}
> \ No newline at end of file
> +}

Yes that is one of the problems with that testcase, however I would like
to see some usleep() in the while body, because otherwise this is
basically bussy loop which would unnecesarilly eat cputime.

The other problem is that most of the aio functions are not modifying
the errno, which is common misconception (the aio_read and aio_write may
return -1 and set the errno if the error happend while the aio operation
was being initalized). So correct test would check the return value from
aio_error() and then check that return value from aio_return() match the
buffer size. Mind that you could call them it exactly once after the
operation is finished.

And seeing the difference between 4-1.c and 4-2.c these two should be
merged into one testcase.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
The demand for IT networking professionals continues to grow, and the
demand for specialized networking skills is growing even more rapidly.
Take a complimentary Learning@Cisco Self-Assessment and learn 
about Cisco certifications, training, and career opportunities. 
http://p.sf.net/sfu/cisco-dev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] aio_fsync: merged 4-1 and 4-2

[Cyril Hrubis]

Hi!
> As Cyril said, seeing the difference between 4-1.c and 4-2.c these two should be
> merged into one testcase.
> 
> Correct test would check the return value from
> aio_error() and then check that return value from aio_return() match the
> buffer size. Mind that you could call them it exactly once after the
> operation is finished.
> 
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: lidan <li.dan@cn.fujitsu.com>
> ---
>  .../conformance/interfaces/aio_fsync/4-1.c         |   16 ++++-
>  .../conformance/interfaces/aio_fsync/4-2.c         |   81 --------------------
>  2 files changed, 14 insertions(+), 83 deletions(-)
>  delete mode 100644 testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-2.c
> 
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-1.c b/testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-1.c
> index 5b965e8..afd19c1 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/aio_fsync/4-1.c
> @@ -27,6 +27,7 @@ int main()
>  #define BUF_SIZE 111
>  	char buf[BUF_SIZE];
>  	int fd;
> +	int ret = 0;
>  	struct aiocb aiocb_write;
>  	struct aiocb aiocb_fsync;
>  
> @@ -68,13 +69,24 @@ int main()
>  		exit(PTS_FAIL);
>  	}
>  
> -	if (aio_error(&aiocb_fsync) < 0)
> +	while ((ret = aio_error(&aiocb_fsync)) == EINPROGRESS)
> +		usleep(10);

I would add a higher value here. At least 10000 (which would still do
the check hundred times in one second). 

> +	if (ret < 0)
>  	{
>  		printf(TNAME " Error at aio_error() : %s\n", strerror(errno));
>  		exit(PTS_FAIL);
>  	}
>  
> +	/* Upon successful completion, fsync() shall return 0.
> +	 * Otherwise, -1 shall be returned and errno set to indicate the error.
> +	 */
> +	if (aio_return(&aiocb_fsync))
> +	{
> +		printf(TNAME " Error at aio_return() : %s\n", strerror(errno));
> +		exit(PTS_FAIL);
> +	}

Ah, right, we check the return value from aio_fsync() which is 0 or -1
(I've thought that we were checking for the return value from
 aio_write() which should be buffer size).

But the in both cases the error number (eg. errno value) is returned by
the aio_error() call. So in the first case you should use strerror(ret)
and it shouldn't be at all in the second printf().

And for the coding style, the opening curly bracket should really be on
the same line as the if(). I know these tests are inconsistent but that
is one more reason to fix them.

>  	close(fd);
>  	printf ("Test PASSED\n");

And please remove the space after printf here.

>  	return PTS_PASS;
> -}
> \ No newline at end of file
> +}

Maybe it would be best to send two patches, one that fixes the coding
style and one that fixes the functionality...

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
The demand for IT networking professionals continues to grow, and the
demand for specialized networking skills is growing even more rapidly.
Take a complimentary Learning@Cisco Self-Assessment and learn 
about Cisco certifications, training, and career opportunities. 
http://p.sf.net/sfu/cisco-dev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] aio_fsync: merged 4-1 and 4-2

[Cyril Hrubis]

Hi!
> If they have some thing else, I will fix and resend on next Monday because
> the weekend is coming now.

Looks good, commited, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
RSA(R) Conference 2012
Save $700 by Nov 18
Register now
http://p.sf.net/sfu/rsa-sfdev2dev1
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list