[Mesa-dev] [PATCH] glx: Properly handle cases where screen creation fails

[Eric Engestrom]

On Wednesday, 2018-02-21 09:48:54 -0500, Chuck Atkins wrote:
> > > -   if (xmdpy->smapi->destroy)
> > > -      xmdpy->smapi->destroy(xmdpy->smapi);
> > > -   free(xmdpy->smapi);
> > > +   if (xmdpy->smapi)
> > > +   {
> > > +      if (xmdpy->smapi->destroy)
> > > +         xmdpy->smapi->destroy(xmdpy->smapi);
> > > +      free(xmdpy->smapi);
> > > +   }
> >
> > I don't know this code so I don't know if the patch is right, but just
> > pointing out this hunk could be written as a simple one-line change:
> >
> > -   if (xmdpy->smapi->destroy)
> > +   if (xmdpy->smapi && xmdpy->smapi->destroy)
> >
> 
> Combining the two would cause  xmdpy->smapi to leak when the
> xmdpy->smapi->destroy callback is null.  This way, destroy get's called
> when it's set but xmdpy->smapi always gets freed.

I think you may have misunderstood me: my "one line change" is *instead
of* your hunk, not *on top of* it. If you don't touch anything else (in
that hunk) then free() is always called :)