On Sun, May 25, 2014 at 09:52:26PM +0200, Pavel Březina wrote:
On 05/23/2014 09:13 PM, Sumit Bose wrote:
On Fri, May 23, 2014 at 07:09:23PM +0200, Pavel Březina wrote:
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:
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?
I'm fine with the name. Do you have time to do this, or shall I prepare a patch on top of yours?
bye, Sumit
Hi, new patches are attached.
I went through the patchset again because it changed a bit from the initial review and in general I think it's too much of code to be reviewed by a single person.
I only have some nitpicks, apart from what Sumit said: sss_sifp_send_message_ex - after dbus_connection_send_with_reply_and_block is called, should we also check reply for being non-NULL? Also, if dbus_error is set but also reply is set, we should unref reply.
In sss_sifp_alloc_zero() you have two spaces: memset(addr, '\0', size * num); ^^
But more importantly, as valgrind reveals, there are some memory leaks, at least in the tests. I don't want to block including the library for the beta because of these leaks, but please file a ticket to clean them up post-beta.