[cairo] [PATCH v2 2/2] test: Fix issues reported by cppcheck static analysis tool

[RAVI NANJUNDAPPA]

Hello Bertram, 
 
If you look at the pdf-mime-data.c file, 'free(data)' is handled by the callee at line numbers (from original file): 108, 153 and 159.
I guess that's the reason, cppcheck tool didn't complain for it. So the proposed changes are fine as per my knowledge. 
Let me know if am missing something here.

Bryce, I've tried to refactor the code to print the appropriate error messages for each error condition in read_file() function.
I've sent the next version of this patch to the mailing list. Please have a look at it and share your opinion.

Thanks and Best Regards, 
N Ravi
------- Original Message -------
Sender : Bryce Harrington<bryce at osg.samsung.com>
Date : Aug 22, 2014 01:52 (GMT+05:30)
Title : Re: [cairo] [PATCH v2 2/2] test: Fix issues reported by cppcheck static analysis tool

On Thu, Aug 21, 2014 at 09:18:04PM +0200, Bertram Felgenhauer wrote:
> Ravi Nanjundappa wrote:
> > @@ -71,11 +71,15 @@ 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);
> >   return CAIRO_TEST_NO_MEMORY;
> > +    }
> >  
> > -    if (fread(*data, *len, 1, fp) != 1)
> > +    if (fread(*data, *len, 1, fp) != 1) {
> > + fclose(fp);
> >   return CAIRO_TEST_FAILURE;
> > +    }
> 
> shouldn't there be a 'free(data)' here, too?

Actually, the caller frees the returned data if the test status is
failed.  However, I agree it'd be cleaner if the function cleaned up
after itself in this case.

Also, all the callers are printing out the same error message, "Could
not read input jpeg file", when actually the function call has four
distinct error conditions.

I'm thinking that read_file() should be made a bit more self-contained
by having it take ownership of printing the appropriate error message
for each error condition, as well as closing files and freeing memory.

Then the calling code can be simplified down to just:

    test_status = read_file (...);
    if (test_status != CAIRO_TEST_SUCCESS)
        return test_status;

Bryce
-- 
cairo mailing list
cairo at cairographics.org
http://lists.cairographics.org/mailman/listinfo/cairo