[PATCH] LDB: Redudant test on NULL context remove

[Jeremy Allison]

On Tue, Apr 19, 2016 at 08:15:51AM +0200, Petr Cech wrote:
> On 04/18/2016 05:27 PM, Simo wrote:
> >On Thu, 2016-03-31 at 10:38 +0200, Volker Lendecke wrote:
> >>On Thu, Mar 31, 2016 at 08:27:28AM +0200, Petr Cech wrote:
> >>>
> >>>For clarity:
> >>>
> >>>original mail is
> >>>https://lists.samba.org/archive/samba-technical/2016-March/113068.h
> >>>tml
> >>>
> >>>the main idea is
> >>>
> >>>"There is redudant test on NULL context in ldb_dn_new_fmt()
> >>>function.
> >>>We use this (NULL) context in talloc_vasprintf() function which is
> >>>able to work with NULL at all. And at the end, we free this newly
> >>>created (by talloc_vasprintf) context. So it should be safe to
> >>>remove
> >>>this check."
> >>If you follow that path fully, even the !ldb check is redundant.
> >>ldb_dn_new will deal with ldb==NULL fine and return NULL.
> >>
> >>By the way, our convention is to add a signed-off-by (git commit -s)
> >>flag to patches. Can you do that?
> >>
> >>You might also internally contact Simo Sorce. He has put in those
> >>checks ages ago, maybe he has some comments on your patches.
> >
> >Patch looks good to me.
> >I think it is ok to keep the check on ldb != NULL, to avoid busywork.
> >
> >Reviwed-by: Simo Sorce <idra at samba.org>
> >
> >Can a second team member add their reviewed-by ?
> >
> >Simo.
> >
> 
> Thanks Simo for review.
> 
> There is patch with signed-off-by flag.

Pushed, thanks !


> Petr^4 Čech

> From 9e63828d7455bcb03f7702114df8777c4473fae4 Mon Sep 17 00:00:00 2001
> From: Petr Cech <pcech at redhat.com>
> Date: Mon, 21 Mar 2016 09:15:17 -0400
> Subject: [PATCH] LDB: Redudant test on NULL context remove
> 
> There is redudant test on NULL context in ldb_dn_new_fmt() function.
> We use this (NULL) context in talloc_vasprintf() function which is
> able to work with NULL at all. And at the end, we free this newly
> created (by talloc_vasprintf) context. So it should be safe to remove
> this check.
> 
> Signed-off-by: Petr Cech <pcech at redhat.com>
> ---
>  lib/ldb/common/ldb_dn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
> index b7d2de7e00fc167f36b475a56d5874f9495d1d27..ab78776368e5693538316e1b28cfdea17b766cc2 100644
> --- a/lib/ldb/common/ldb_dn.c
> +++ b/lib/ldb/common/ldb_dn.c
> @@ -171,7 +171,7 @@ struct ldb_dn *ldb_dn_new_fmt(TALLOC_CTX *mem_ctx,
>  	char *strdn;
>  	va_list ap;
>  
> -	if ( (! mem_ctx) || (! ldb)) return NULL;
> +	if (! ldb) return NULL;
>  
>  	va_start(ap, new_fmt);
>  	strdn = talloc_vasprintf(mem_ctx, new_fmt, ap);
> -- 
> 2.5.5
>