Updated patch

[John (J5) Palmieri]

On Mon, 2006-04-17 at 21:50 -0400, Tim Moloney wrote:
> John (J5) Palmieri wrote:
> > On Mon, 2006-04-17 at 15:50 -0400, Tim Moloney wrote:
> >   
> >> On Mon 2005-12-05 at 13:56:07 PST, Havoc Pennington wrote:
> >>
> >>  > On Mon, 2005-12-05 at 13:33 +0000, Matthew Johnson wrote:
> >>  >>
> >>  >> 8853: assertion failed "(*(const char*)_DBUS_FUNCTION_NAME) != '_'" 
> >> file "dbus-connection.c" line 2877 function 
> >> _dbus_connection_read_write_dispatch
> >>  >>
> >>  >> Which appears to enforce that _dbus_return_val_if_fail can only be
> >>  >> called from functions that don't start with an underscore. I have no
> >>  >> idea why this is, but have accordingly moved the calls to that into the
> >>  >> wrapper functions.
> >>  >
> >>  > The underscore functions are internal/private, and return_if_fail should
> >>  > only be used to check correctness of arguments to public API. For
> >>  > private API, just use dbus_assert() (and be sure public API that calls
> >>  > the private API has return_if_fail so we don't hit the assert when
> >>  > checks are enabled)
> >>
> >> I found the above in the list archive.
> >>
> >> If I understand it correctly, then the asserts are incorrect in 
> >> dbus_connection_read_write, dbus_connection_read_write_dispatch, and 
> >> _dbus_connection_read_write_dispatch.  I've attached a patch that I 
> >> believe corrects the problem.  I think this is what Matthew Johnson said 
> >> he did to get these functions working.  This patch works on my FC5 
> >> system but should be tested more thoroughly.
> >>
> >> On a related note, the FC5 spec file is producing code with the asserts 
> >> enabled although the spec file appears to be only enabling the asserts 
> >> for a 'make check' run.  I mention this in case anyone here has 
> >> influence over the FC5 spec file (and can figure out why it's enabling 
> >> asserts).
> >>     
> >
> > To catch errors. I don't think I will take them out until we hit 1.0.
> >
> >   
> The asserts weren't taken out.  They were moved from the one internal 
> work method (see above, where Havoc Pennington said they don't belong) 
> to the two wrapper methods (where he said they do belong) which are the 
> only two methods that calls the internal work method.
> 
> I don't believe there is a change in function or loss of capability.  
> Without the patch, I don't see how dbus_connection_read_write and 
> dbus_connection_read_write_dispatch can ever work with asserts enabled. 
> 
> The patch corrects a problem which is unfortunately compounded by the 
> broken FC5 spec file.

I was referring to the spec file in my last comment.  No it is not
broken.  The asserts are turned on to catch errors - in this case an
error with the assert code itself but more importantly to catch what
could be major bugs before we go 1.0.  Patch looks good.  Thanks.

-- 
John (J5) Palmieri <johnp at redhat.com>