CTDB: Small patch set fixing memory leak and other things for ctdb_daemon_read_cb

Martin Schwenke martin at meltin.net
Wed Mar 21 05:55:36 UTC 2018


On Tue, 20 Mar 2018 08:39:36 +0100, Swen Schillig <swen at vnet.ibm.com>
wrote:

> On Tue, 2018-03-20 at 12:26 +1100, Martin Schwenke wrote:
> > On Mon, 19 Mar 2018 10:33:25 +0100, Swen Schillig <swen at vnet.ibm.com>
> > wrote:
> >   
> > > On Mon, 2018-03-19 at 14:35 +1100, Martin Schwenke via samba-
> > > technical
> > > wrote:  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > You're right.
> > > I need to add that one check back.  
> > 
> > Excellent.
> > 
> > As I said in my original comment, I think you should name the caller
> > (i.e. queue_process()) in the commit message.  That gives someone
> > less
> > familiar with the code a starting point for a review.  They can go
> > straight to queue_process() and see how the context in which the
> > callback is called. If they want to they can also look more broadly
> > with the file that contains it and confirm that the associated setup
> > function is indeed used to setup the queue.
> > 
> > I also vaguely feel like the space/wrapping cleanups don't need to be
> > in the same commits as the other stuff.  However, I'm not going to
> > bike-shed on that. Perhaps others will feel more strongly.
> > 
> > With a commit message that names the calling function:
> > 
> > Reviewed-by: Martin Schwenke <martin at meltin.net>
> >   
>  [...]  
>  [...]  
> > > Removed the cleanup.  
> > 
> > OK, but I think that understanding what I meant would be
> > valuable.  ;-)

> Updated patch-set.

Signed-off-by: Martin Schwenke <martin at meltin.net>

Another team reviewer, please?

peace & happiness,
martin



More information about the samba-technical mailing list