dbus_connection_send_with_reply_and_block and errors

[Timo Teräs]

ext Colin Walters wrote:
> On Tue, 2005-08-09 at 19:55 +0300, Timo Teräs wrote:
> 
>>ext Havoc Pennington wrote:
>>
>>>On Tue, 2005-08-09 at 19:02 +0300, Timo Teräs wrote:
>>>
>>>>dbus_set_error() never duplicates the error.name field; it just does a
>>>>pointer assignment.
>>>
>>>I think this is the bug (I'm not sure why it is like this to be honest,
>>>seems like a pretty bad idea, though it looks like it was deliberate)
>>
>>I guess in most cases the error.name is a constant and can be safely 
>>assigned. This makes sense since it saves memory and CPU cycles. But in case 
>>of dbus_set_error_from_message() it is just broken.
>>
>>So, shall I write a patch to do duplication of .name in dbus_set_error() and 
>>do a free() in dbus_error_free()?
> 
> Hm...doesn't this make OOM handling significantly more complicated if
> setting an error can require memory allocation?  

Not sure about this. But dbus_set_error does already allocate memory for
the message field. I guess in OOM situation dbus_set_error_const()
should be used without exception.

> Perhaps instead of the reference counting (which sounds complicated) we
> could simply to add a "dbus_bool_t free_name" member;
> dbus_set_error_from_message sets this (and dups the name), and
> dbus_error_free frees the name if it's set.  Wouldn't that work?

Yes. Now that I thinked more about this, it might be good to do the
change in dbus_set_error() since most people will propably assume that
it dups both fields. Atleast I thought so until I read the docs in more
detail. So I agree with Havoc that doing assignment of .name in
dbus_set_error() is a bad idea.

I'll do a patch for this and post it asap for review.

Cheers,
  Timo