On 05/22/2014 09:00 PM, 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.
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.
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...
But then again a method may return a reply so we would have to wrap also dbus_message_get_args at least for basic types a that brings us deeper and deeper and sifp will at some point have to become a full dbus interface, which was not the intention.
Keeping those function and the dbus dependency makes the code using sifp cleaner even if you need a little bit extended functionality, since you can call any method through dbus but you don't haven to initialize the connection, handle reply even if it is not needed, etc.
So you can keep using sifp to simply fetch and parse object attributes and to call List* and Find* methods but you still can call any other supported method and reuse the sifp dbus connection.