Re: [PATCH 1/3] Remove useless NULL checks before calling free(3) in gettext.c

[christos%astron.com@localhost (Christos Zoulas)]

In article <1432396450-5296-1-git-send-email-will%worrbase.com@localhost>,
William Orr  <will%worrbase.com@localhost> wrote:
>---
> src/lib/libintl/gettext.c | 54 ++++++++++++++++++-----------------------------
> 1 file changed, 21 insertions(+), 33 deletions(-)
>
>diff --git a/src/lib/libintl/gettext.c b/src/lib/libintl/gettext.c
>index 6591422..479b4be 100644
>--- a/src/lib/libintl/gettext.c
>+++ b/src/lib/libintl/gettext.c
>@@ -661,12 +661,12 @@ free_sysdep_table(struct mosysdepstr_h **table,
>uint32_t nstring)
> {
> 	uint32_t i;
> 
>+	if (! table)
>+		return;
>+
> 	for (i=0; i<nstring; i++) {

I'd fix the whitespace here since we are rewriting most of it and use
a c99 for loop index...

>-		if (table[i]) {

The above test is needed, otherewise your free(table[i]->expanded);
will dereference NULL and crash.

>-			if (table[i]->expanded)
>-				free(table[i]->expanded);
>-			free(table[i]);
>-		}
>+		free(table[i]->expanded);
>+		free(table[i]);
> 	}
> 	free(table);
> }
>@@ -680,26 +680,16 @@ unmapit(struct domainbinding *db)
> 	if (mohandle->addr && mohandle->addr != MAP_FAILED)
> 		munmap(mohandle->addr, mohandle->len);
> 	mohandle->addr = NULL;
>-	if (mohandle->mo.mo_otable)
>-		free(mohandle->mo.mo_otable);
>-	if (mohandle->mo.mo_ttable)
>-		free(mohandle->mo.mo_ttable);
>-	if (mohandle->mo.mo_charset)
>-		free(mohandle->mo.mo_charset);
>-	if (mohandle->mo.mo_htable)
>-		free(mohandle->mo.mo_htable);
>-	if (mohandle->mo.mo_sysdep_segs)
>-		free(mohandle->mo.mo_sysdep_segs);
>-	if (mohandle->mo.mo_sysdep_otable) {
>-		free_sysdep_table(mohandle->mo.mo_sysdep_otable,
>-				  mohandle->mo.mo_sysdep_nstring);
>-	}
>-	if (mohandle->mo.mo_sysdep_ttable) {
>-		free_sysdep_table(mohandle->mo.mo_sysdep_ttable,
>-				  mohandle->mo.mo_sysdep_nstring);
>-	}
>-	if (mohandle->mo.mo_plural)
>-		_gettext_free_plural(mohandle->mo.mo_plural);
>+	free(mohandle->mo.mo_otable);
>+	free(mohandle->mo.mo_ttable);
>+	free(mohandle->mo.mo_charset);
>+	free(mohandle->mo.mo_htable);
>+	free(mohandle->mo.mo_sysdep_segs);
>+	free_sysdep_table(mohandle->mo.mo_sysdep_otable,
>+			  mohandle->mo.mo_sysdep_nstring);
>+	free_sysdep_table(mohandle->mo.mo_sysdep_ttable,
>+			  mohandle->mo.mo_sysdep_nstring);
>+	_gettext_free_plural(mohandle->mo.mo_plural);

I'd use KNF indent for the second line indents above.

> 	memset(&mohandle->mo, 0, sizeof(mohandle->mo));
> 	return 0;
> }
>@@ -918,17 +908,15 @@ dcngettext(const char *domainname, const char
>*msgid1, const char *msgid2,
> 	    domainname, db) == NULL)
> 		goto fail;
> 
>-	if (odomainname)
>-		free(odomainname);
>-	if (ocname)
>-		free(ocname);
>+	free(odomainname);
>+	free(ocname);
>+
> 	odomainname = strdup(domainname);
> 	ocname = strdup(cname);
> 	if (!odomainname || !ocname) {
>-		if (odomainname)
>-			free(odomainname);
>-		if (ocname)
>-			free(ocname);
>+		free(odomainname);
>+		free(ocname);
>+
> 		odomainname = ocname = NULL;
> 	}
> 	else
>-- 
>2.1.0
>
>