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(a)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(a)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.