[PATCH] Use talloc_autofree_context() to allocate global memory

[Kamen Mazdrashki]

Hi Volker, Andrew,

Please find attached updated patch set.

Updates:
patch 0002: debug_list_class_names_and_levels() accepts memory context to
perform allocations in
                    Andrew, I do totally agree with your comment.
patch 0005: Allocates struct bitmap using service pointer as a memory
context. I have traced all call to
                   this function and as far as I can tell, pservice
parameter is always a valid memory context.
                   Nevertheless, I have also added a short comment that it
*must* be valid memory context.
patch 0006: Totally a cosmetic patch to add blocks explicitly according to
coding style. Please feel free
                    to skip/NAK it. It doesn't add any value (except for
coding) anyways.

For quick review, patches are on the web too:
https://github.com/kamenim/samba/commits/util-debug-use-autofree-context
(nobody said anything about github as quick review tool, but I still
blindly push there too :)

Cheers,
Kamen

On Tue, Sep 9, 2014 at 3:03 AM, Andrew Bartlett <abartlet at samba.org> wrote:

> On Mon, 2014-09-08 at 14:22 +0200, Kamen Mazdrashki wrote:
> > Hi,
> >
> >
> > Please find attached several patches to move global structure off of
> > NULL
> > context into autofree context.
> > I have hit those while running unit tests in Openchange.
> >
> >
> > Strictly speaking, patch 0002 is not needed as the caller for this
> > function
> > uses the memory returned and frees it immediately - we have only 1
> > caller
> > at the moment.
> > Nevertheless, we can't assume what the caller of
> > debug_list_class_names_and_levels()
> > is going to do and hence this patch. May be it will be good if we pass
> > memory context explicitly?
> >
> >
> > Please review.
>
> Thanks Kamen,
>
> I'm trying to remember why we went a bit cold on
> talloc_autofree_context().  I know we got into trouble when it involved
> destructors.
>
> That said, changes like the one in
>
> [PATCH 5/5] loadparm: Allocate bitmap structures in autofree context
>
> are probably wrong.  We should use the pservice as a talloc parent,
> unless there is a good reason.
>
> I also think that if you have to write an explanation like the one in:
> [PATCH 2/5] util/debug: debug_list_class_names_and_levels - alloc mem
>  off of autofree context
>
> that this means the patch is wrong - talloc_autofree_context() should be
> the very last resort, not the first, because it hides exactly the bugs
> you are worried about.
>
> > A quick summary of changes:
> >  lib/ldb/common/ldb_modules.c |  6 +++---
> >  lib/param/loadparm.c         |  2 +-
> >  lib/tevent/tevent.c          |  2 +-
> >  lib/util/debug.c             | 12 +++++++++---
> >  4 files changed, 14 insertions(+), 8 deletions(-)
> >
> >
> > patches also pushed at:
> > https://github.com/kamenim/samba/commits/util-debug-use-autofree-context
> >
> > (probing what will be the feedback for Github :)
> >
> >
> > Cheers,
> > kamen
>
> --
> Andrew Bartlett
> http://samba.org/~abartlet/
> Authentication Developer, Samba Team  http://samba.org
> Samba Developer, Catalyst IT
> http://catalyst.net.nz/services/samba
>
>
>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-util-debug-Use-talloc_autofree_context-instead-of-NU.patch
Type: text/x-patch
Size: 1414 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140910/3b6d8ab3/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-util-debug-debug_list_class_names_and_levels-accept-.patch
Type: text/x-patch
Size: 3248 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140910/3b6d8ab3/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-ldb_modules-use-talloc-autofree-context-to-allocate-.patch
Type: text/x-patch
Size: 1511 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140910/3b6d8ab3/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-tevent-Allocate-tevent_ops_list-structures-in-autofr.patch
Type: text/x-patch
Size: 871 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140910/3b6d8ab3/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-loadparm-Allocate-service-copymap-in-service-memory-.patch
Type: text/x-patch
Size: 1371 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140910/3b6d8ab3/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-loadparm-init_copymap-Add-braces-around-if-for-block.patch
Type: text/x-patch
Size: 1038 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140910/3b6d8ab3/attachment-0005.bin>