This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix up return codes for tests in tst-ftell-active-handler


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/10/2014 08:16 AM, Siddhesh Poyarekar wrote:
> Hi,
> 
> The test functions used a variable ret to store failure codes for
> individual tests, but the variable was incorrectly used to record
> other failure codes too, resulting in overwriting of the tests status.
> This is now fixed by making sure that the ret variable is used only
> for recording test failures.
> 
> Siddhesh
> 
> 	* libio/tst-ftell-active-handler.c (do_ftell_test): Don't mix
> 	up test status with function return status.
> 	(do_write_test): Likewise.
> 	(do_append_test): Likewise.

Looks good to me.

> ---
>  libio/tst-ftell-active-handler.c | 49 +++++++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/libio/tst-ftell-active-handler.c b/libio/tst-ftell-active-handler.c
> index 54bfe63..5d5fc26 100644
> --- a/libio/tst-ftell-active-handler.c
> +++ b/libio/tst-ftell-active-handler.c
> @@ -119,17 +119,20 @@ do_ftell_test (const char *filename)
>  	{
>  	  FILE *fp;
>  	  int fd;
> +	  int fileret;
> +

OK.

>  	  printf ("\tftell: %s (file, \"%s\"): ", j == 0 ? "fdopen" : "fopen",
>  		  test_modes[i].mode);
>  
>  	  if (j == 0)
> -	    ret = get_handles_fdopen (filename, fd, fp, test_modes[i].fd_mode,
> -				      test_modes[i].mode);
> +	    fileret = get_handles_fdopen (filename, fd, fp,
> +					  test_modes[i].fd_mode,
> +					  test_modes[i].mode);

OK.

>  	  else
> -	    ret = get_handles_fopen (filename, fd, fp, test_modes[i].mode);
> +	    fileret = get_handles_fopen (filename, fd, fp, test_modes[i].mode);

OK.

>  
> -	  if (ret != 0)
> -	    return ret;
> +	  if (fileret != 0)
> +	    return fileret;

OK.

So far I don't see any mixing of error returns.

If the error is non-zero then we return from do_ftell_test.

>  
>  	  long off = ftell (fp);
>  	  if (off != test_modes[i].old_off)
> @@ -143,7 +146,12 @@ do_ftell_test (const char *filename)
>  
>  	  /* The effect of this write on the offset should be seen in the ftell
>  	     call that follows it.  */
> -	  int ret = write (fd, data, data_len);
> +	  int write_ret = write (fd, data, data_len);

OK. Alright this is bad since it can overwrite ret set above e.g. ret |= 1;

> +	  if (write_ret != data_len)
> +	    {
> +	      printf ("write failed (%m)\n");
> +	      ret |= 1;
> +	    }

OK. Good check.

>  	  off = ftell (fp);
>  
>  	  if (off != test_modes[i].new_off)
> @@ -184,21 +192,23 @@ do_write_test (const char *filename)
>      {
>        for (int i = 0; i < sizeof (test_modes) / sizeof (struct test); i++)
>  	{
> +	  int fileret;

OK.

>  	  printf ("\twrite: %s (file, \"%s\"): ", j == 0 ? "fopen" : "fdopen",
>  		  test_modes[i].mode);
>  
>  	  if (j == 0)
> -	    ret = get_handles_fopen (filename, fd, fp, test_modes[i].mode);
> +	    fileret = get_handles_fopen (filename, fd, fp, test_modes[i].mode);
>  	  else
> -	    ret = get_handles_fdopen (filename, fd, fp, test_modes[i].fd_mode,
> -				      test_modes[i].mode);
> +	    fileret = get_handles_fdopen (filename, fd, fp,
> +					  test_modes[i].fd_mode,
> +					  test_modes[i].mode);


OK.

>  
> -	  if (ret != 0)
> -	    return ret;
> +	  if (fileret != 0)
> +	    return fileret;

OK.

>  
>  	  /* Move offset to just before the end of the file.  */
> -	  off_t ret = lseek (fd, file_len - 1, SEEK_SET);
> -	  if (ret == -1)
> +	  off_t seek_ret = lseek (fd, file_len - 1, SEEK_SET);
> +	  if (seek_ret == -1)

OK.

>  	    {
>  	      printf ("lseek failed: %m\n");
>  	      ret |= 1;
> @@ -258,17 +268,20 @@ do_append_test (const char *filename)
>      {
>        for (int i = 0; i < sizeof (test_modes) / sizeof (struct test); i++)
>  	{
> +	  int fileret;
> +

OK.

>  	  printf ("\tappend: %s (file, \"%s\"): ", j == 0 ? "fopen" : "fdopen",
>  		  test_modes[i].mode);
>  
>  	  if (j == 0)
> -	    ret = get_handles_fopen (filename, fd, fp, test_modes[i].mode);
> +	    fileret = get_handles_fopen (filename, fd, fp, test_modes[i].mode);

OK.

>  	  else
> -	    ret = get_handles_fdopen (filename, fd, fp, test_modes[i].fd_mode,
> -				      test_modes[i].mode);
> +	    fileret = get_handles_fdopen (filename, fd, fp,
> +					  test_modes[i].fd_mode,
> +					  test_modes[i].mode);

OK.

>  
> -	  if (ret != 0)
> -	    return ret;
> +	  if (fileret != 0)
> +	    return fileret;

OK.

>  
>  	  /* Write some data.  */
>  	  size_t written = fputs_func (data, fp);
> 

Cheers,
Carlos.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTI4tjAAoJECXvCkNsKkr/p2EH/Rn5xYXK9hYrOrSAzSCi+P8c
wPxXmG+smAVTbmPk212j2w2/xZr2WWqYdr1eDBp/dHQYEpyDcbeTSNFj/dmppyvS
BxHCP4ALPDnRS19IXyrmRfDIHUVFvP8vR2OVoBZ2taMtl9YpfqL6g8uOYA3pJEal
d8jlfJhCQW34vCyFojsyx7yYGJcK6putoMUIe+g7DSnkiKH3RAMFyHL80ZKFPSXK
8XDbI0lHFNO/XWxebLIXLZjrQajDmgTdkc4ySucqqXLr9AjJGgYdZzOOyNZIb1Bz
2pSTOhrorqCJNtgu2FR0VX+Ll6/WhidhVIBdidhKWs6WsrVWqdcP1lcWzNHK11A=
=jGR0
-----END PGP SIGNATURE-----


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]