[SSSD] [PATCH] SBUS: Generate introspection from the interface meta

Pavel Březina pbrezina at redhat.com
Tue Apr 15 07:24:26 UTC 2014


On 04/14/2014 04:17 PM, Jakub Hrozek wrote:
> On Fri, Apr 11, 2014 at 12:53:08PM +0200, Pavel Březina wrote:
>> On 04/10/2014 11:03 AM, Jakub Hrozek wrote:
>>> The attached patch implements introspecting the sbus interfaces as
>>> tracked by #2234.
>>>
>>> There is one part of the patch I dislike, but I wanted to get other
>>> opinions, too -- the discard_const in sbus_message_handler(). I was
>>> going back and forth on whether sbus_introspect() should be an sbus
>>> handler like the other handlers or whether I should special case it (and
>>> then pass the interface directly and not through a void pointer).
>>> Special casing the introspection might be a bit cleaner, but would cause
>>> some duplication in sbus_message_handler().
>>
>> In my opinion it is correct to make it a standard sbus handler. I
>> think that it is ok to use discard_const() in this situation,
>> however it can be avoided. Cleaner solution would be to create a new
>> context for private data, e.g.:
>> struct sbus_introspect_ctx {
>>      const struct sbus_interface_meta *iface;
>> };
>>
>>> The other question I have is whether there should be a way to mark an
>>> interface as not introspectable? I can't think of a reason, so by
>>> default all interfaces can be introspected.
>>
>> We should deal with this question when we actually hit an interface
>> that must not be introspectable for some reason.
>>
>>> The introspection also doesn't generate DocString annotations yet, I
>>> will send a patch for that separately.
>>
>> The code looks good and it works splendidly! Good job!
>>
>> I'm leaving the discard_const issue on your decision, I don't think
>> it is really necessary to avoid it in this case.
>>
>> It's an ack from me.
>
> Hi,
>
> I think the sbus_introspect_ctx context is a good idea. It requires some
> special casing in sbus_message_handler(), though..
>
> Attached are two patches, one is the modified patch including the
> sbus_introspect_ctx, the other is just the interdiff for easier review.
>
> Thanks for the review!

Ack.




More information about the sssd-devel mailing list