Re: [PATCH v4 2/2] test: Fix issues reported by cppcheck static analysis tool

On Mon, Aug 25, 2014 at 09:28:40AM +0530, Ravi Nanjundappa wrote:
> cppcheck analysis tool reports the following issues when run on the
> latest Cairo source.
> 
> $ grep "(error)"  cppcheck_error_log.txt
> [test/pdf-mime-data.c:58]: (error) Possible null pointer dereference: file - otherwise it is redundant to check if file is null at line 53
> [test/pdf-mime-data.c:75]: (error) Resource leak: fp
> $
> 
> The proposed changes fixes the above issues.

Looks good, merged both patches in series.
 
> And also it does some refactoring to print the appropriate error messages
> for each error condition in read_file() function and also to free the allocated
> data buffer.
> 
> Signed-off-by: Ravi Nanjundappa <[email protected]>
> Reviewed-by: Chris Wilson <[email protected]>
> ---
>  test/pdf-mime-data.c |   25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/test/pdf-mime-data.c b/test/pdf-mime-data.c
> index e2c529e..fd5af1f 100644
> --- a/test/pdf-mime-data.c
> +++ b/test/pdf-mime-data.c
> @@ -50,7 +50,7 @@ read_file (const cairo_test_context_t *ctx,
>      FILE *fp;
>  
>      fp = fopen (file, "rb");
> -    if (file == NULL) {
> +    if (fp == NULL) {
>  	char filename[4096];
>  
>  	/* try again with srcdir */
> @@ -61,8 +61,11 @@ read_file (const cairo_test_context_t *ctx,
>      if (fp == NULL) {
>  	switch (errno) {
>  	case ENOMEM:
> +	    cairo_test_log (ctx, "Could not create file handle for %s due to \
> +				lack of memory\n", file);
>  	    return CAIRO_TEST_NO_MEMORY;
>  	default:
> +	    cairo_test_log (ctx, "Could not get the file handle for %s\n", file);
>  	    return CAIRO_TEST_FAILURE;
>  	}
>      }
> @@ -71,11 +74,19 @@ read_file (const cairo_test_context_t *ctx,
>      *len = ftell(fp);
>      fseek (fp, 0, SEEK_SET);
>      *data = malloc (*len);
> -    if (*data == NULL)
> +    if (*data == NULL) {
> +	fclose(fp);
> +	cairo_test_log (ctx, "Could not allocate memory for buffer to read \
> +				from file %s\n", file);
>  	return CAIRO_TEST_NO_MEMORY;
> +    }
>  
> -    if (fread(*data, *len, 1, fp) != 1)
> +    if (fread(*data, *len, 1, fp) != 1) {
> +	free (data);
> +	fclose(fp);
> +	cairo_test_log (ctx, "Could not read data from file %s\n", file);
>  	return CAIRO_TEST_FAILURE;
> +    }
>  
>      fclose(fp);
>      return CAIRO_TEST_SUCCESS;
> @@ -104,8 +115,6 @@ preamble (cairo_test_context_t *ctx)
>      image = cairo_image_surface_create_from_png (IMAGE_FILE ".png");
>      test_status = read_file (ctx, IMAGE_FILE ".jpg", &data, &len);
>      if (test_status) {
> -	cairo_test_log (ctx, "Could not read input jpeg file %s\n", IMAGE_FILE ".jpg");
> -	free(data);
>  	return test_status;
>      }
>  
> @@ -149,17 +158,11 @@ preamble (cairo_test_context_t *ctx)
>  
>      test_status = read_file (ctx, IMAGE_FILE ".jpg", &data, &len);
>      if (test_status) {
> -	cairo_test_log (ctx, "Could not read input jpeg file %s\n", IMAGE_FILE ".jpg");
> -	free(data);
>  	return test_status;
>      }
>  
>      test_status = read_file (ctx, CAIRO_TEST_OUTPUT_DIR "/" BASENAME "-000.jpg", &out_data, &out_len);
>      if (test_status) {
> -	free (data);
> -	cairo_test_log (ctx,
> -			"Could not read input jpeg file %s\n",
> -			CAIRO_TEST_OUTPUT_DIR "/" BASENAME "-000.jpg");
>  	return test_status;
>      }
>  
> -- 
> 1.7.9.5
> 
> -- 
> cairo mailing list
> [email protected]
> http://lists.cairographics.org/mailman/listinfo/cairo
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo