[3/3] snapshot: Don't use extra after it's been freed (CID #220086)

Submitted by Bryce Harrington on June 9, 2018, 5:34 a.m.

Details

Message ID 1528522467-8873-4-git-send-email-bryce@bryceharrington.org
State New
Series "Coverity fixes"
Headers show

Commit Message

Bryce Harrington June 9, 2018, 5:34 a.m.
Coverity ID: 220086

Signed-off-by: Bryce Harrington <bryce@bryceharrington.org>
---
 src/cairo-surface-snapshot.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

diff --git a/src/cairo-surface-snapshot.c b/src/cairo-surface-snapshot.c
index c8f3078..0dace49 100644
--- a/src/cairo-surface-snapshot.c
+++ b/src/cairo-surface-snapshot.c
@@ -108,6 +108,7 @@  _cairo_surface_snapshot_acquire_source_image (void                    *abstract_
     if (unlikely (status)) {
 	cairo_surface_destroy (extra->target);
 	free (extra);
+	extra = NULL;
     }
 
     *extra_out = extra;

Comments

Uli Schlachter June 9, 2018, 7 a.m.
I guess the intended semantics is that the value of *extra_out is
undefined if an error is returned. You are changing this to always NULL
this value instead. Thus, could you also make the earlier NO_MEMORY
return in this function do *extra_out = NULL;?

On 09.06.2018 07:34, Bryce Harrington wrote:
> Coverity ID: 220086
> 
> Signed-off-by: Bryce Harrington <bryce@bryceharrington.org>
> ---
>  src/cairo-surface-snapshot.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/cairo-surface-snapshot.c b/src/cairo-surface-snapshot.c
> index c8f3078..0dace49 100644
> --- a/src/cairo-surface-snapshot.c
> +++ b/src/cairo-surface-snapshot.c
> @@ -108,6 +108,7 @@ _cairo_surface_snapshot_acquire_source_image (void                    *abstract_
>      if (unlikely (status)) {
>  	cairo_surface_destroy (extra->target);
>  	free (extra);
> +	extra = NULL;
>      }
>  
>      *extra_out = extra;
>
Bryce Harrington June 12, 2018, 9:59 p.m.
On Sat, Jun 09, 2018 at 09:00:30AM +0200, Uli Schlachter wrote:
> I guess the intended semantics is that the value of *extra_out is
> undefined if an error is returned. You are changing this to always NULL
> this value instead. Thus, could you also make the earlier NO_MEMORY
> return in this function do *extra_out = NULL;?

Yep, good call.

I'll send out a v2 of this patchset shortly.

Bryce

> On 09.06.2018 07:34, Bryce Harrington wrote:
> > Coverity ID: 220086
> > 
> > Signed-off-by: Bryce Harrington <bryce@bryceharrington.org>
> > ---
> >  src/cairo-surface-snapshot.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/cairo-surface-snapshot.c b/src/cairo-surface-snapshot.c
> > index c8f3078..0dace49 100644
> > --- a/src/cairo-surface-snapshot.c
> > +++ b/src/cairo-surface-snapshot.c
> > @@ -108,6 +108,7 @@ _cairo_surface_snapshot_acquire_source_image (void                    *abstract_
> >      if (unlikely (status)) {
> >  	cairo_surface_destroy (extra->target);
> >  	free (extra);
> > +	extra = NULL;
> >      }
> >  
> >      *extra_out = extra;
> > 
> 
> 
> -- 
> Bruce Schneier can read and understand Perl programs.