xserver: Branch 'master'

[Peter Hutterer]

On Sun, Aug 16, 2009 at 06:01:49PM +0100, Daniel Stone wrote:
> Hi,
> I know this isn't your fault, but thought I might point it out anyway
> ...
> 
> On Thu, Aug 13, 2009 at 06:02:13PM -0700, Peter Hutterer wrote:
> >  out_ctx:
> >      dbus_error_free(&error);
>        ^^^^^^^^^^^^^^^^^^^^^^^^
> >  
> > -    if (info->hal_ctx) {
> > -        if (!libhal_ctx_shutdown(info->hal_ctx, &error)) {
> > -            LogMessage(X_WARNING, "config/hal: couldn't shut down context: %s (%s)\n",
> > -		       error.name ? error.name : "unknown error",
> > -		       error.message ? error.message : "null");
> > -            dbus_error_free(&error);
> > -        }
> > -        libhal_ctx_free(info->hal_ctx);
> > +    if (!libhal_ctx_shutdown(info->hal_ctx, &error)) {
> > +        LogMessage(X_WARNING, "config/hal: couldn't shut down context: %s (%s)\n",
> > +                error.name ? error.name : "unknown error",
> > +                error.message ? error.message : "null");
> > +        dbus_error_free(&error);
> >      }
> >  
> >  out_err:
> >      dbus_error_free(&error);
>        ^^^^^^^^^^^^^^^^^^^^^^^^
> >  
> > +    if (info->hal_ctx) {
> > +        libhal_ctx_free(info->hal_ctx);
> > +    }
> > +
> >      info->hal_ctx = NULL;
> >      info->system_bus = NULL;
> 
> That's going to end in tears.  There's no need to free it at the top of
> out_ctx, and indeed it means, if anything, that you really don't want to
> be passing it into libhal_ctx_shutdown.

the dbus API claims that dbus_error_free "Frees an error that's been set (or
just initialized), then reinitializes the error as in dbus_error_init()."
http://dbus.freedesktop.org/doc/api/html/group__DBusErrors.html

all the cases that go to out_ctx have the error set, and passing in an
already set error into dbus stuff is bad. Anything I'm missing here?

(this patch has been tested on suse, fedora and I think remi tested it on
gentoo as well)

Cheers,
  Peter