[SSSD] [PATCH] libsss_dbus

Pavel Březina pbrezina at redhat.com
Fri May 23 17:09:23 UTC 2014


On 05/23/2014 07:06 PM, Sumit Bose wrote:
> On Thu, May 22, 2014 at 09:00:49PM +0200, Pavel Březina wrote:
>> On 05/22/2014 11:17 AM, Sumit Bose wrote:
>>> On Thu, May 22, 2014 at 11:07:51AM +0200, Sumit Bose wrote:
>>>> On Mon, May 19, 2014 at 10:40:40PM +0200, Pavel Březina wrote:
>>>>> On 05/16/2014 03:32 PM, Jakub Hrozek wrote:
>>>>>> On Fri, May 16, 2014 at 02:54:55PM +0200, Pavel Březina wrote:
>>>>>>> On 05/16/2014 02:04 PM, Jakub Hrozek wrote:
>>>>>>>> On Thu, May 15, 2014 at 09:23:46PM +0200, Pavel Březina wrote:
>>>>>>>>> On 05/15/2014 09:09 PM, Pavel Březina wrote:
>>>>>
>>>>
>>>> ...
>>>>
>>>>
>>>>> All concerns should be fixed (but the memory leak in test). I also changed
>>>>> the prefix to sss_sifp as Sumit asked (sending as separate patch).
>>>>
>>>> Thank you, imo it more looks much more consistent.
>>>>
>>>> Please find attached a small patch which should illustrate my other
>>>> suggestion to remove the build time DBus dependency for the user of the
>>>> library. Doc fixes and a free function for the new struct are still
>>>> missing, the patch should just illustrate the idea. If you think this
>>>> makes sense, I can add the missing parts if you like.
>>>>
>>>> With this the dbus-1 requirement in the pc file can be removed as well
>>>> because the runtime dependency for DBus will be solved by package
>>>> management. Btw, I think there is an issue in the current pc file, both
>>>> requirements should be in a single line.
>>>
>>> and now with the patch ...
>>
>> Well... the way it is now, this encapsulation makes the public
>> sss_sifp_create_message() and sss_sifp_send_message() useless since you
>> cannot append parameters to the message.
>
> ah, ok, I see, so the purpose was to make DBusMessage available to the
> caller.
>
>>
>> If you really dislike the dependency that much, we can just remove these
>> functions. I like them since they made the code a little simpler, even when
>> you append the arguments through dbus function. But I can go without them.
>
> I have to admit I would like to remove them, but maybe have have
> different use cases in mind. I'm looking at case like:
>
>      err = sss_sifp_init(&ctx);
>      if (err != SSS_SIFP_OK) {
>          return 2;
>      }
>
>      err = sss_sifp_fetch_user_by_name(ctx, username, &user);
>      if (err != SSS_SIFP_OK) {
>          return 3;
>      }
>
>      err = sss_sifp_find_attr_as_string(user->attrs, attr, &str);
>      if (err != SSS_SIFP_OK) {
>          return 4;
>      }
>
>      printf("[%s]=[%s]\n", attr, str);
>
>      sss_sifp_free_object(ctx, &user);
>      sss_sifp_free(&ctx);
>
>
> Maybe we can split the header file like libcurl does so that the caller can
> decided if he prefers sss_sifp_easy.h where dbus includes file are not required
> or the more flexible sss_sifp.h ?

Ok, we can do that. How about moving the functions that requires dbus 
headers into sss_sifp_dbus.h?

>
> bye,
> Sumit
>>
>> Or we need also to provide equivalent to dbus_message_append_args(). For
>> example we can go one more step forward and also pass the dbus arguments in
>> the sss_sifp_create_message(), like:
>>
>> sss_sifp_create_message(ctx, path, SSS_DBUS_IFACE_COMPONENTS,
>>                          "ChangeDebugLevel",
>>                          DBUS_TYPE_UINT32, &level,
>>                          DBUS_TYPE_INVALID);
>>
>> But having it like this would really require conversion from DBUS_TYPE into
>> SSS_SIFP_ATTR_TYPE which I wanted to avoid (in the beginning I didn't plan
>> to use the custom types at all). It would make the library more than what I
>> intended from the beginning. However it may not be a bad thing after all...
>>
>> Your call?
>>
>> _______________________________________________
>> sssd-devel mailing list
>> sssd-devel at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>




More information about the sssd-devel mailing list