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...
Your call?