[PATCH] Fix wrong talloc context, remove queue_destructor and minor updates.

Martin Schwenke martin at meltin.net
Tue Dec 12 04:37:57 UTC 2017


Hi Swen,

On Mon, 11 Dec 2017 09:44:04 +0100, Swen Schillig via samba-technical
<samba-technical at lists.samba.org> wrote:

> This is my first patch to SAMBA.
> I'm quiet sure it's easy stuff but I'm trying to gently introduce
> myself to the community.
> Please go easy on me if I failed already some basics at the beginning.

Most of this looks valid.  Can you please split it up into a commit per
logical change?  Putting each logical change into separate commit makes
things much clearer and also gives you more opportunity to explain each
change in more detail.

Comments:

* I don't think removing the talloc context argument is valid.  There
  is no guarantee that private_data points to a talloc context, so you
  can't depend on that even though it is true for every current use.  I
  can't imagine a circumstance where private_data is not a talloc
  context... but still...  :-)

* In the commit message for the destructor removal you could point to
  the commit that contains the removal of the only use of the
  "destroyed" structure member, which was really the only reason for
  the existence of the destructor.

peace & happiness,
martin



More information about the samba-technical mailing list