svn commit: samba r14257 - in trunk/source/passdb: .

[Andrew Bartlett]

On Sun, 2006-03-12 at 19:22 -0500, simo wrote:
> On Sun, 2006-03-12 at 16:18 -0800, Jeremy Allison wrote:
> > On Sun, Mar 12, 2006 at 07:12:03PM -0500, simo wrote:
> > > On Sun, 2006-03-12 at 15:48 -0800, Jeremy Allison wrote:
> > > > On Sun, Mar 12, 2006 at 11:09:32PM +0000, idra at samba.org wrote:
> > > > > Author: idra
> > > > > Date: 2006-03-12 23:09:31 +0000 (Sun, 12 Mar 2006)
> > > > > New Revision: 14257
> > > > > 
> > > > > WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=14257
> > > > > 
> > > > > Log:
> > > > > 
> > > > > commit some fixes to the previous patch as Volker pointed out some flaws.
> > > > 
> > > > Still has problems. *Never* use talloc_free, always use TALLOC_FREE.
> > > > If you're using talloc_free you need to be re-examining your
> > > > patch.
> > > 
> > > no, the use of talloc_free() is ok because we are always sure the
> > > context passed is not null and valid.
> > 
> > It's a habit thing. If you *always* use TALLOC_FREE you'll have
> > less bugs than if you think you can get away with talloc_free
> > as 'it doesn't matter this time'. I just fixed an unbelievably
> > subtle bug where someone used a _free call instead of a _FREE
> > style call. No one visually inspecting the code would have (or
> > did) see it. Coverity found it.
> 
> Yes, I'm following the streams of patches. Do you know what I thought
> when I saw it?
> "That would not have happened if we had a talloc hierarchy".

Agreed.  I certainly hope we don't see this in Samba4.  It would be
better to get the talloc_free() asserts, and find where the hierarchy
has been broken.

> > I know it doesn't matter in this case, but using TALLOC_FREE
> > instead here doesn't hurt either, and so it's safe to just
> > always use TALLOC_FREE.
> 
> See my argumentation on another mail/
> 
> > If I do a grep on the source and see talloc_free, instead
> > of TALLOC_FREE, I want it to be an exceptional case *only*
> > with a comment explaining why that was used.
> 
> But I see you feel strongly (reading another reply while answering) ..
> in that case why not just make talloc_free check for context not being
> null itself instead of adding a really ugly (visually because of all
> caps) macro ? :-)

talloc_free() (and indeed free(), no most platforms) of NULL is a valid
no-op.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Student Network Administrator, Hawker College  http://hawkerc.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20060313/b0002c9b/attachment.bin