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