[Freeipa-devel] [PATCH] remove useless if-before-free tests

[Jim Meyering]

I've been on a crusade (;-) to remove useless if-before-free tests,
so ran a script that spotted some here.  I think I removed the first
batch (without braces) automatically, then manually removed the ones
with curly braces around the free statements.

You may well have doubts about the portability of removing those
tests, but as long as you don't care about SunOS4 or earlier, you'll
be fine.  I've done similar things for e.g., coreutils, glibc, and git,
and have had no problems.

FYI, here's the script to detect them (with comments showing
how to remove some mechanically):

    http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=build-aux/useless-if-before-free

Here's the patch:

>From 3106dd7fdf85796de7caaf5e58ffd4c2343bc681 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Sun, 4 May 2008 15:44:32 +0200
Subject: [PATCH] remove useless if-before-free tests

---
 ipa-client/ipa-getkeytab.c                         |    2 +-
 ipa-server/ipa-kpasswd/ipa_kpasswd.c               |    8 ++--
 .../ipa-pwd-extop/ipa_pwd_extop.c                  |   36 ++++++-------------
 3 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/ipa-client/ipa-getkeytab.c b/ipa-client/ipa-getkeytab.c
index 9642650..8a8c856 100644
--- a/ipa-client/ipa-getkeytab.c
+++ b/ipa-client/ipa-getkeytab.c
@@ -433,7 +433,7 @@ error_out:
 	if (res) ldap_msgfree(res);
 	if (ld) ldap_unbind_ext(ld, NULL, NULL);
 	if (control) ber_bvfree(control);
-	if (encs) free(encs);
+	free(encs);
 	return 0;
 }

diff --git a/ipa-server/ipa-kpasswd/ipa_kpasswd.c b/ipa-server/ipa-kpasswd/ipa_kpasswd.c
index 86aa6c1..877fd93 100644
--- a/ipa-server/ipa-kpasswd/ipa_kpasswd.c
+++ b/ipa-server/ipa-kpasswd/ipa_kpasswd.c
@@ -645,9 +645,9 @@ done:
 	if (sctrl) ber_free(sctrl, 1);
 	if (srvctrl) ldap_controls_free(srvctrl);
 	if (res) ldap_msgfree(res);
-	if (exterr1) free(exterr1);
-	if (exterr2) free(exterr2);
-	if (userdn) free(userdn);
+	free(exterr1);
+	free(exterr2);
+	free(userdn);
 	if (ld) ldap_unbind_ext(ld, NULL, NULL);
 	if (tmp_file) {
 		unlink(tmp_file);
@@ -975,7 +975,7 @@ kpreply:
 	*replen = replylen;

 done:
-	if (result_string) free(result_string);
+	free(result_string);
 	if (auth_context) krb5_auth_con_free(context, auth_context);
 	if (kprincpw) krb5_free_principal(context, kprincpw);
 	if (krep.length) free(krep.data);
diff --git a/ipa-server/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/ipa-server/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index dec772c..2bfa517 100644
--- a/ipa-server/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/ipa-server/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -198,20 +198,14 @@ static void ipapwd_keyset_free(struct ipapwd_keyset **pkset)

 	for (i = 0; i < kset->num_keys; i++) {
 		if (kset->keys[i].salt) {
-			if (kset->keys[i].salt->value.bv_val) {
-				free(kset->keys[i].salt->value.bv_val);
-			}
+			free(kset->keys[i].salt->value.bv_val);
 			free(kset->keys[i].salt);
 		}
 		if (kset->keys[i].ekey) {
-			if (kset->keys[i].ekey->value.bv_val) {
-				free(kset->keys[i].ekey->value.bv_val);
-			}
+			free(kset->keys[i].ekey->value.bv_val);
 			free(kset->keys[i].ekey);
 		}
-		if (kset->keys[i].s2kparams.bv_val) {
-			free(kset->keys[i].s2kparams.bv_val);
-		}
+		free(kset->keys[i].s2kparams.bv_val);
 	}
 	free(kset->keys);
 	free(kset);
@@ -238,20 +232,14 @@ static int filter_keys(struct ipapwd_keyset *kset)

 			/* free key */
 			if (kset->keys[i].ekey) {
-				if (kset->keys[i].ekey->value.bv_val) {
-					free(kset->keys[i].ekey->value.bv_val);
-				}
+				free(kset->keys[i].ekey->value.bv_val);
 				free(kset->keys[i].ekey);
 			}
 			if (kset->keys[i].salt) {
-				if (kset->keys[i].salt->value.bv_val) {
-					free(kset->keys[i].salt->value.bv_val);
-				}
+				free(kset->keys[i].salt->value.bv_val);
 				free(kset->keys[i].salt);
 			}
-			if (kset->keys[i].s2kparams.bv_val) {
-				free(kset->keys[i].s2kparams.bv_val);
-			}
+			free(kset->keys[i].s2kparams.bv_val);

 			/* move all remaining keys up by one */
 			kset->num_keys -= 1;
@@ -741,7 +729,7 @@ enc_error:
 	if (kset) ipapwd_keyset_free(&kset);
 	krb5_free_principal(krbctx, princ);
 	if (bval) ber_bvfree(bval);
-	if (svals) free(svals);
+	free(svals);
 	return NULL;
 }

@@ -2543,7 +2531,7 @@ static int ipapwd_setkeytab(Slapi_PBlock *pb)

 	/* Free anything that we allocated above */
 free_and_return:
-	if (serviceName) free(serviceName);
+	free(serviceName);
 	if (kset) ipapwd_keyset_free(&kset);

 	if (bval) ber_bvfree(bval);
@@ -2789,10 +2777,10 @@ static int ipapwd_getConfig(krb5_context krbctx, const char *realm_dn)
 free_and_error:
 	if (mkey) ber_bvfree(mkey);
 	if (be) ber_free(be, 1);
-	if (config->pref_encsalts) free(config->pref_encsalts);
-	if (config->supp_encsalts) free(config->supp_encsalts);
-	if (config->kmkey) free(config->kmkey);
-	if (config) free(config);
+	free(config->pref_encsalts);
+	free(config->supp_encsalts);
+	free(config->kmkey);
+	free(config);
 	if (realm_entry) slapi_entry_free(realm_entry);
 	return LDAP_OPERATIONS_ERROR;
 }
--
1.5.5.1.216.g33c73