[PATCH] Fix Bug 5917 - Samba does not work on site with Read Only Domain Controller

[Jeremy Allison]

On Wed, Sep 04, 2013 at 12:41:16PM +0200, Volker Lendecke wrote:
> On Tue, Sep 03, 2013 at 02:25:57PM -0700, Jeremy Allison wrote:
> > On Wed, Sep 04, 2013 at 09:17:42AM +1200, Andrew Bartlett wrote:
> > > 
> > > Thanks.  While we are looking at additional changes, any chance of
> > > changing this (for master) to use talloc?
> > > 
> > > malloc()/free() is so 1990's ;-)
> > 
> > Yeah, but this sits on top of gencache_XX, which is a
> > bigger change from malloc->talloc than I want to bite
> > off right now :-).
> 
> Here's a patchset that might be the basis for that. It also
> fixes panics in two gencache error paths.
> 
> Please review&push.

The following is the patch I'm not too keen on :

> From 320d82e4809b77cd4b420478376a622475a78dc0 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 4 Sep 2013 08:57:59 +0200
> Subject: [PATCH 6/6] lib: Use "mem_ctx" arg in gencache_get
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/lib/gencache.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
> index 1ecaad5..35f5d1b 100644
> --- a/source3/lib/gencache.c
> +++ b/source3/lib/gencache.c
> @@ -711,7 +711,7 @@ bool gencache_get(const char *keystr, TALLOC_CTX *mem_ctx, char **value,
>  	DATA_BLOB blob;
>  	bool ret = False;
>  
> -	ret = gencache_get_data_blob(keystr, NULL, &blob, ptimeout, NULL);
> +	ret = gencache_get_data_blob(keystr, mem_ctx, &blob, ptimeout, NULL);
>  	if (!ret) {
>  		return false;
>  	}
> @@ -725,6 +725,10 @@ bool gencache_get(const char *keystr, TALLOC_CTX *mem_ctx, char **value,
>  		return false;
>  	}
>  	if (value) {
> +		if (mem_ctx) {
> +			*value = talloc_move(mem_ctx, (char **)&blob.data);
> +			return true;
> +		}
>  		*value = SMB_STRDUP((char *)blob.data);
>  		data_blob_free(&blob);
>  		if (*value == NULL) {

It leaves gencache_get() as a mixed interface, sometimes
returning a talloced value, sometimes a malloced.

I'd really rather fix this to always return a talloced
value, and fix up all the callers to use TALLOC_FREE()
to complete this patchset before pushing.

I don't want to end up in the same place we were with
the pull string interfaces which were mixed for a while,
that way lies disaster.

Jeremy.