[PATCH] dbus: Don't pass NULL message ptr to dbus_message_new_error()

[Dan Williams]

On Wed, 2011-07-27 at 14:06 -0500, Dan Williams wrote:
> On Wed, 2011-07-27 at 10:08 -0700, Chinchilla, Angie V wrote:
> > 
> > > -----Original Message-----
> > > From: Dan Williams [mailto:dcbw at redhat.com]
> > > Sent: Wednesday, July 27, 2011 9:47 AM
> > > To: Chinchilla, Angie V
> > > Cc: hostap at lists.shmoo.com
> > > Subject: Re: [PATCH] dbus: Don't pass NULL message ptr to
> > > dbus_message_new_error()
> > > 
> > > On Mon, 2011-07-25 at 16:48 -0700, Angie Chinchilla wrote:
> > > > Found at least 10 code paths that mistakenly assume
> > > > message is not NULL before invoking
> > > > dbus_message_new_error(message, DBUS_ERROR_NO_MEMORY, NULL).
> > > > Created a wrapper method wpas_dbus_error_no_memory() that
> > > > checks for this case and returns NULL if needed so we don't
> > > > have if (!message) return NULL else return dbus_message_new_error()
> > > > blocks scattered all over the code. Also added a couple of log
> > > > messages to the path that hits this error so the (now gracefully
> > > > handled) errors don't fail silently.
> > > 
> > > I've actually just significantly reworked the new dbus interface code
> > > to
> > > fix this problem and clean it up a bunch.  So Jouni, if you don't mind
> > > waiting for my patch for this issue, that'd be great and we can make a
> > > determination then.
> > > 
> > > Dan
> > > 
> > 
> > I'm fine with any solution that cleans up the issue. There are also the two (see following patch hunks) memory leaks that should be cleaned up as well. I'll submit a separate patch for those if they don't get cleaned up by your rework.
> 
> They should; these functions don't create DBusMessages anymore.

The patch is big, and it's being held for moderator approval, so until
Jouni lets it through here's a direct link:

http://people.redhat.com/dcbw/0001-dbus-getset-cleanup-08.patch

Dan

> Dan
> 
> > Thanks,
> > Angie
> > 
> > > > @@ -647,6 +654,8 @@ static void put_changed_properties(const struct
> > > wpa_dbus_object_desc *obj_dsc,
> > > >  			wpa_printf(MSG_ERROR, "dbus: %s: Cannot get new value
> > > "
> > > >  				   "of property %s", __func__,
> > > >  				   dsc->dbus_property);
> > > > +			if (getter_reply)
> > > > +				dbus_message_unref(getter_reply);
> > > >  			continue;
> > > >  		}
> > > >
> > > > @@ -670,6 +679,9 @@ static void put_changed_properties(const struct
> > > wpa_dbus_object_desc *obj_dsc,
> > > >  	return;
> > > >
> > > >  err:
> > > > +	if (getter_reply)
> > > > +		dbus_message_unref(getter_reply);
> > > > +
> > > >  	wpa_printf(MSG_ERROR, "dbus: %s: Cannot construct signal",
> > > __func__);
> > > >  }
> > > >
> > > 
> > 
> 
> 
> _______________________________________________
> HostAP mailing list
> HostAP at lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap