[SSSD] [PATCH] DBus: Automatic pack/unpack of method handler arguments

Pavel Březina pbrezina at redhat.com
Fri Apr 18 13:31:17 UTC 2014


On 04/18/2014 10:59 AM, Jakub Hrozek wrote:
> On Fri, Apr 18, 2014 at 10:37:57AM +0200, Pavel Březina wrote:
>> On 04/17/2014 05:17 PM, Jakub Hrozek wrote:
>>> On Thu, Apr 17, 2014 at 02:46:06PM +0200, Pavel Březina wrote:
>>>> On 03/20/2014 08:11 AM, Stef Walter wrote:
>>>>> On 14.03.2014 23:00, Stef Walter wrote:
>>>>>> Here's the next set of DBus patches. This implements automatic packing
>>>>>> and unpacking of arguments for method handlers.
>>>>>
>>>>> Rebased on master. Also the last patches had incorrectly generated files
>>>>> due to a rebase.
>>>>>
>>>>> Stef
>>>>
>>>> Hi,
>>>> I'm sorry for the delay. Can you rebase the patches one more time please?
>>>
>>> I took the liberty of rebasing the patches on Stef's behalf as the only
>>> conflict was in the Introspection piece I wrote. The first patch (a new
>>> one) silences the warning about undefined method.
>>>
>>> The second and third are what used to be first and second previously.
>>
>> Thanks.
>>
>> First patch should be the last one, since invoker field is not
>> present before Stef's patches.
>
> I've actually added the invoker in patch #2. The zero is vtable offset.
> It's not very clean, though, so I'm even considering including the
> introspection method into all the vtables..but I don't think we should
> block including these patches.

Ah, ok. I missed that.

>>
>> 0001: Ack, but maybe amend the commit message that it fixes the warning.
>
> Done.
>
>>
>> 0002: C-code ack... I didn't go through the Python code though, I
>> left that for someone more experienced with Python.
>
> I went through the patches as well. Ack++, the Python code looks good.
> I've done one more change I didn't notice until I was reviewing the
> patches -- the ifp.xml file should explicitly include a RawHandler for
> the 'ping' method. I've fixed that now and attached a new patchset.

Ack.

>
>>
>> 0003: Ack.
>
> Ack++
>
>>
>> Smoke tests went fine.
>>
>> Great work Stef!
>>
>> I'm attaching two more patches that fixes few warnings. Feel free to
>> squash them into Stef's first patch.
>
>>  From c50ca1fca02a1e5d4c2d7571b27ad5171192c7b8 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>> Date: Fri, 18 Apr 2014 10:17:20 +0200
>> Subject: [PATCH 4/5] sbus_tests: fix missing invoker in initializer
>
> ACK
>
>>  From 00601045be47cfa4504e5f02fe456027163e56ef Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>> Date: Fri, 18 Apr 2014 10:26:14 +0200
>> Subject: [PATCH 5/5] sbus request: fix error initialization
>>
>> fixes:
>> sssd_dbus_request.c:28:1: error: missing initializer [-Werror=missing-field-initializers]
>> sssd_dbus_request.c:28:1: error: (near initialization for 'error_internal.dummy1') [-Werror=missing-field-initializers]
>> ---
>
> I'm not opposed to pushing this patch, but I think the C warning is
> spurious. Especially when using 'static', I would have expected all the
> fields not set explicitly to be zeroed out. Also, I had to disable
> missing-field-initializers in my CFLAGS, because I've seen many warnings
> like this with the autogenerated code.

Interesting, this was the only one I saw. I guess it's time to update 
from my prehistoric distro :)

> But your change is fine, too, I think, so if you agree with adding the
> RawHandler to IFP's 'ping', I'll just push the patches.

Ack to all.




More information about the sssd-devel mailing list