[PATCH] resource release cleanup in net/

[PATCH] resource release cleanup in net/

[Jesper Juhl]

Hi David,

Since Andrew merged the patch that makes calling crypto_free_tfm() with a 
NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for 
NULL before calling that function, and while I was at it I removed similar 
redundant checks before calls to kfree() and vfree() in the same files. 
There are also a few tiny whitespace cleanups in there.

I believe you are the maintainer in charge of the code I've changed, so 
that's why I'm sending this to you. Sorry about the long CC list, but I 
wanted to make sure that people whos code I've changed got informed.

Since the changes I'm making are very similar for both kfree, vfree & 
crypto_free_tfm, I've lumped it all into one single patch. If you'd prefer 
it split into 3-4 patches instead then just let me know and I can do that 
instead.

If the patch is acceptable then please let me know if you'll merge it into 
your tree or if you'd like me to send on to Andrew myself for future -mm 
inclusion.
If the patch is not OK, then feedback is of course very welcome.


Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
---

 drivers/net/wireless/airo.c                     |   95 ++++++++++--------------
 drivers/net/wireless/hostap/hostap_crypt_ccmp.c |    5 -
 drivers/net/wireless/hostap/hostap_crypt_tkip.c |   10 +-
 drivers/net/wireless/hostap/hostap_crypt_wep.c  |    5 -
 net/ieee80211/ieee80211_crypt_ccmp.c            |    5 -
 net/ieee80211/ieee80211_crypt_tkip.c            |   10 +-
 net/ieee80211/ieee80211_crypt_wep.c             |    5 -
 net/ipv4/ah4.c                                  |   18 +---
 net/ipv4/esp4.c                                 |   28 +++----
 net/ipv4/ipcomp.c                               |    6 -
 net/ipv6/addrconf.c                             |    9 --
 net/ipv6/ah6.c                                  |   28 ++-----
 net/ipv6/esp6.c                                 |   28 +++----
 net/ipv6/ipcomp6.c                              |    9 --
 net/sctp/endpointola.c                          |    3
 net/sctp/socket.c                               |    3
 net/sunrpc/auth_gss/gss_krb5_crypto.c           |    3
 net/sunrpc/auth_gss/gss_krb5_mech.c             |    9 --
 net/sunrpc/auth_gss/gss_spkm3_mech.c            |   16 +---
 19 files changed, 119 insertions(+), 176 deletions(-)

--- linux-2.6.12-rc3-mm1-orig/drivers/net/wireless/airo.c	2005-04-30 18:37:01.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/airo.c	2005-04-30 19:55:57.000000000 +0200
@@ -2383,14 +2383,10 @@ void stop_airo_card( struct net_device *
 			dev_kfree_skb(skb);
 	}
 
-	if (ai->flash)
-		kfree(ai->flash);
-	if (ai->rssi)
-		kfree(ai->rssi);
-	if (ai->APList)
-		kfree(ai->APList);
-	if (ai->SSID)
-		kfree(ai->SSID);
+	kfree(ai->flash);
+	kfree(ai->rssi);
+	kfree(ai->APList);
+	kfree(ai->SSID);
 	if (freeres) {
 		/* PCMCIA frees this stuff, so only for PCI and ISA */
 	        release_region( dev->base_addr, 64 );
@@ -2406,8 +2402,7 @@ void stop_airo_card( struct net_device *
 		}
         }
 #ifdef MICSUPPORT
-	if (ai->tfm)
-		crypto_free_tfm(ai->tfm);
+	crypto_free_tfm(ai->tfm);
 #endif
 	del_airo_dev( dev );
 	free_netdev( dev );
@@ -3627,11 +3622,9 @@ static u16 setup_card(struct airo_info *
 	WepKeyRid wkr;
 	int rc;
 
-	memset( &mySsid, 0, sizeof( mySsid ) );
-	if (ai->flash) {
-		kfree (ai->flash);
-		ai->flash = NULL;
-	}
+	memset(&mySsid, 0, sizeof(mySsid));
+	kfree(ai->flash);
+	ai->flash = NULL;
 
 	/* The NOP is the first step in getting the card going */
 	cmd.cmd = NOP;
@@ -3668,14 +3661,11 @@ static u16 setup_card(struct airo_info *
 		tdsRssiRid rssi_rid;
 		CapabilityRid cap_rid;
 
-		if (ai->APList) {
-			kfree(ai->APList);
-			ai->APList = NULL;
-		}
-		if (ai->SSID) {
-			kfree(ai->SSID);
-			ai->SSID = NULL;
-		}
+		kfree(ai->APList);
+		ai->APList = NULL;
+		kfree(ai->SSID);
+		ai->SSID = NULL;
+
 		// general configuration (read/modify/write)
 		status = readConfigRid(ai, lock);
 		if ( status != SUCCESS ) return ERROR;
@@ -3689,10 +3679,8 @@ static u16 setup_card(struct airo_info *
 				memcpy(ai->rssi, (u8*)&rssi_rid + 2, 512);
 		}
 		else {
-			if (ai->rssi) {
-				kfree(ai->rssi);
-				ai->rssi = NULL;
-			}
+			kfree(ai->rssi);
+			ai->rssi = NULL;
 			if (cap_rid.softCap & 8)
 				ai->config.rmode |= RXMODE_NORMALIZED_RSSI;
 			else
@@ -4553,7 +4541,7 @@ static int proc_status_open( struct inod
 	memset(file->private_data, 0, sizeof(struct proc_data));
 	data = (struct proc_data *)file->private_data;
 	if ((data->rbuffer = kmalloc( 2048, GFP_KERNEL )) == NULL) {
-		kfree (file->private_data);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 
@@ -4633,7 +4621,7 @@ static int proc_stats_rid_open( struct i
 	memset(file->private_data, 0, sizeof(struct proc_data));
 	data = (struct proc_data *)file->private_data;
 	if ((data->rbuffer = kmalloc( 4096, GFP_KERNEL )) == NULL) {
-		kfree (file->private_data);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 
@@ -4899,12 +4887,12 @@ static int proc_config_open( struct inod
 	memset(file->private_data, 0, sizeof(struct proc_data));
 	data = (struct proc_data *)file->private_data;
 	if ((data->rbuffer = kmalloc( 2048, GFP_KERNEL )) == NULL) {
-		kfree (file->private_data);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	if ((data->wbuffer = kmalloc( 2048, GFP_KERNEL )) == NULL) {
-		kfree (data->rbuffer);
-		kfree (file->private_data);
+		kfree(data->rbuffer);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	memset( data->wbuffer, 0, 2048 );
@@ -5174,15 +5162,15 @@ static int proc_wepkey_open( struct inod
 	memset(&wkr, 0, sizeof(wkr));
 	data = (struct proc_data *)file->private_data;
 	if ((data->rbuffer = kmalloc( 180, GFP_KERNEL )) == NULL) {
-		kfree (file->private_data);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	memset(data->rbuffer, 0, 180);
 	data->writelen = 0;
 	data->maxwritelen = 80;
 	if ((data->wbuffer = kmalloc( 80, GFP_KERNEL )) == NULL) {
-		kfree (data->rbuffer);
-		kfree (file->private_data);
+		kfree(data->rbuffer);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	memset( data->wbuffer, 0, 80 );
@@ -5221,14 +5209,14 @@ static int proc_SSID_open( struct inode 
 	memset(file->private_data, 0, sizeof(struct proc_data));
 	data = (struct proc_data *)file->private_data;
 	if ((data->rbuffer = kmalloc( 104, GFP_KERNEL )) == NULL) {
-		kfree (file->private_data);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	data->writelen = 0;
 	data->maxwritelen = 33*3;
 	if ((data->wbuffer = kmalloc( 33*3, GFP_KERNEL )) == NULL) {
-		kfree (data->rbuffer);
-		kfree (file->private_data);
+		kfree(data->rbuffer);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	memset( data->wbuffer, 0, 33*3 );
@@ -5265,14 +5253,14 @@ static int proc_APList_open( struct inod
 	memset(file->private_data, 0, sizeof(struct proc_data));
 	data = (struct proc_data *)file->private_data;
 	if ((data->rbuffer = kmalloc( 104, GFP_KERNEL )) == NULL) {
-		kfree (file->private_data);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	data->writelen = 0;
 	data->maxwritelen = 4*6*3;
 	if ((data->wbuffer = kmalloc( data->maxwritelen, GFP_KERNEL )) == NULL) {
-		kfree (data->rbuffer);
-		kfree (file->private_data);
+		kfree(data->rbuffer);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	memset( data->wbuffer, 0, data->maxwritelen );
@@ -5315,7 +5303,7 @@ static int proc_BSSList_open( struct ino
 	memset(file->private_data, 0, sizeof(struct proc_data));
 	data = (struct proc_data *)file->private_data;
 	if ((data->rbuffer = kmalloc( 1024, GFP_KERNEL )) == NULL) {
-		kfree (file->private_data);
+		kfree(file->private_data);
 		return -ENOMEM;
 	}
 	data->writelen = 0;
@@ -5372,10 +5360,11 @@ static int proc_BSSList_open( struct ino
 static int proc_close( struct inode *inode, struct file *file )
 {
 	struct proc_data *data = (struct proc_data *)file->private_data;
-	if ( data->on_close != NULL ) data->on_close( inode, file );
-	if ( data->rbuffer ) kfree( data->rbuffer );
-	if ( data->wbuffer ) kfree( data->wbuffer );
-	kfree( data );
+	if (data->on_close)
+		data->on_close(inode, file);
+	kfree(data->rbuffer);
+	kfree(data->wbuffer);
+	kfree(data);
 	return 0;
 }
 
@@ -7269,10 +7258,10 @@ static int readrids(struct net_device *d
 	len = comp->len;
 
 	if (copy_to_user(comp->data, iobuf, min(len, (int)RIDSIZE))) {
-		kfree (iobuf);
+		kfree(iobuf);
 		return -EFAULT;
 	}
-	kfree (iobuf);
+	kfree(iobuf);
 	return 0;
 }
 
@@ -7350,10 +7339,10 @@ static int writerids(struct net_device *
 
 		if (copy_to_user(comp->data, iobuf,
 				 min((int)comp->len, (int)RIDSIZE))) {
-			kfree (iobuf);
+			kfree(iobuf);
 			return -EFAULT;
 		}
-		kfree (iobuf);
+		kfree(iobuf);
 		return 0;
 
 	default:
@@ -7366,7 +7355,7 @@ static int writerids(struct net_device *
 		return -ENOMEM;
 
 	if (copy_from_user(iobuf,comp->data,comp->len)) {
-		kfree (iobuf);
+		kfree(iobuf);
 		return -EFAULT;
 	}
 
@@ -7383,10 +7372,10 @@ static int writerids(struct net_device *
 	}
 
 	if((*writer)(ai, ridcode, iobuf,comp->len,1)) {
-		kfree (iobuf);
+		kfree(iobuf);
 		return -EIO;
 	}
-	kfree (iobuf);
+	kfree(iobuf);
 	return 0;
 }
 
--- linux-2.6.12-rc3-mm1-orig/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-04-30 18:37:01.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-04-30 19:41:06.000000000 +0200
@@ -109,8 +109,7 @@ static void * hostap_ccmp_init(int key_i
 
 fail:
 	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
+		crypto_free_tfm(priv->tfm);
 		kfree(priv);
 	}
 	module_put(THIS_MODULE);
@@ -121,7 +120,7 @@ fail:
 static void hostap_ccmp_deinit(void *priv)
 {
 	struct hostap_ccmp_data *_priv = priv;
-	if (_priv && _priv->tfm)
+	if (_priv)
 		crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 	module_put(THIS_MODULE);
--- linux-2.6.12-rc3-mm1-orig/drivers/net/wireless/hostap/hostap_crypt_tkip.c	2005-04-30 18:37:01.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_tkip.c	2005-04-30 19:41:06.000000000 +0200
@@ -102,10 +102,8 @@ static void * hostap_tkip_init(int key_i
 
 fail:
 	if (priv) {
-		if (priv->tfm_michael)
-			crypto_free_tfm(priv->tfm_michael);
-		if (priv->tfm_arc4)
-			crypto_free_tfm(priv->tfm_arc4);
+		crypto_free_tfm(priv->tfm_michael);
+		crypto_free_tfm(priv->tfm_arc4);
 		kfree(priv);
 	}
 	module_put(THIS_MODULE);
@@ -116,10 +114,10 @@ fail:
 static void hostap_tkip_deinit(void *priv)
 {
 	struct hostap_tkip_data *_priv = priv;
-	if (_priv && _priv->tfm_michael)
+	if (_priv) {
 		crypto_free_tfm(_priv->tfm_michael);
-	if (_priv && _priv->tfm_arc4)
 		crypto_free_tfm(_priv->tfm_arc4);
+	}
 	kfree(priv);
 	module_put(THIS_MODULE);
 }
--- linux-2.6.12-rc3-mm1-orig/drivers/net/wireless/hostap/hostap_crypt_wep.c	2005-04-30 18:37:01.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_wep.c	2005-04-30 19:41:06.000000000 +0200
@@ -69,8 +69,7 @@ static void * prism2_wep_init(int keyidx
 
 fail:
 	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
+		crypto_free_tfm(priv->tfm);
 		kfree(priv);
 	}
 	module_put(THIS_MODULE);
@@ -81,7 +80,7 @@ fail:
 static void prism2_wep_deinit(void *priv)
 {
 	struct prism2_wep_data *_priv = priv;
-	if (_priv && _priv->tfm)
+	if (_priv)
 		crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 	module_put(THIS_MODULE);
--- linux-2.6.12-rc3-mm1-orig/net/ieee80211/ieee80211_crypt_ccmp.c	2005-04-30 18:37:16.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_ccmp.c	2005-04-30 19:41:06.000000000 +0200
@@ -96,8 +96,7 @@ static void * ieee80211_ccmp_init(int ke
 
 fail:
 	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
+		crypto_free_tfm(priv->tfm);
 		kfree(priv);
 	}
 
@@ -108,7 +107,7 @@ fail:
 static void ieee80211_ccmp_deinit(void *priv)
 {
 	struct ieee80211_ccmp_data *_priv = priv;
-	if (_priv && _priv->tfm)
+	if (_priv)
 		crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 }
--- linux-2.6.12-rc3-mm1-orig/net/ieee80211/ieee80211_crypt_tkip.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_tkip.c	2005-04-30 19:41:06.000000000 +0200
@@ -90,10 +90,8 @@ static void * ieee80211_tkip_init(int ke
 
 fail:
 	if (priv) {
-		if (priv->tfm_michael)
-			crypto_free_tfm(priv->tfm_michael);
-		if (priv->tfm_arc4)
-			crypto_free_tfm(priv->tfm_arc4);
+		crypto_free_tfm(priv->tfm_michael);
+		crypto_free_tfm(priv->tfm_arc4);
 		kfree(priv);
 	}
 
@@ -104,10 +102,10 @@ fail:
 static void ieee80211_tkip_deinit(void *priv)
 {
 	struct ieee80211_tkip_data *_priv = priv;
-	if (_priv && _priv->tfm_michael)
+	if (_priv) {
 		crypto_free_tfm(_priv->tfm_michael);
-	if (_priv && _priv->tfm_arc4)
 		crypto_free_tfm(_priv->tfm_arc4);
+	}
 	kfree(priv);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/ieee80211/ieee80211_crypt_wep.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_wep.c	2005-04-30 19:41:06.000000000 +0200
@@ -64,8 +64,7 @@ static void * prism2_wep_init(int keyidx
 
 fail:
 	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
+		crypto_free_tfm(priv->tfm);
 		kfree(priv);
 	}
 	return NULL;
@@ -75,7 +74,7 @@ fail:
 static void prism2_wep_deinit(void *priv)
 {
 	struct prism2_wep_data *_priv = priv;
-	if (_priv && _priv->tfm)
+	if (_priv)
 		crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 }
--- linux-2.6.12-rc3-mm1-orig/net/ipv4/ah4.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/ah4.c	2005-04-30 19:41:06.000000000 +0200
@@ -263,10 +263,8 @@ static int ah_init_state(struct xfrm_sta
 
 error:
 	if (ahp) {
-		if (ahp->work_icv)
-			kfree(ahp->work_icv);
-		if (ahp->tfm)
-			crypto_free_tfm(ahp->tfm);
+		kfree(ahp->work_icv);
+		crypto_free_tfm(ahp->tfm);
 		kfree(ahp);
 	}
 	return -EINVAL;
@@ -279,14 +277,10 @@ static void ah_destroy(struct xfrm_state
 	if (!ahp)
 		return;
 
-	if (ahp->work_icv) {
-		kfree(ahp->work_icv);
-		ahp->work_icv = NULL;
-	}
-	if (ahp->tfm) {
-		crypto_free_tfm(ahp->tfm);
-		ahp->tfm = NULL;
-	}
+	kfree(ahp->work_icv);
+	ahp->work_icv = NULL;
+	crypto_free_tfm(ahp->tfm);
+	ahp->tfm = NULL;
 	kfree(ahp);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/ipv4/esp4.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/esp4.c	2005-04-30 19:41:06.000000000 +0200
@@ -343,22 +343,18 @@ static void esp_destroy(struct xfrm_stat
 	if (!esp)
 		return;
 
-	if (esp->conf.tfm) {
-		crypto_free_tfm(esp->conf.tfm);
-		esp->conf.tfm = NULL;
-	}
-	if (esp->conf.ivec) {
-		kfree(esp->conf.ivec);
-		esp->conf.ivec = NULL;
-	}
-	if (esp->auth.tfm) {
-		crypto_free_tfm(esp->auth.tfm);
-		esp->auth.tfm = NULL;
-	}
-	if (esp->auth.work_icv) {
-		kfree(esp->auth.work_icv);
-		esp->auth.work_icv = NULL;
-	}
+	crypto_free_tfm(esp->conf.tfm);
+	esp->conf.tfm = NULL;
+
+	kfree(esp->conf.ivec);
+	esp->conf.ivec = NULL;
+
+	crypto_free_tfm(esp->auth.tfm);
+	esp->auth.tfm = NULL;
+
+	kfree(esp->auth.work_icv);
+	esp->auth.work_icv = NULL;
+
 	kfree(esp);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/ipv4/ipcomp.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/ipcomp.c	2005-04-30 20:04:42.000000000 +0200
@@ -296,8 +296,7 @@ static void ipcomp_free_scratches(void)
 
 	for_each_cpu(i) {
 		void *scratch = *per_cpu_ptr(scratches, i);
-		if (scratch)
-			vfree(scratch);
+		vfree(scratch);
 	}
 
 	free_percpu(scratches);
@@ -350,8 +349,7 @@ static void ipcomp_free_tfms(struct cryp
 
 	for_each_cpu(cpu) {
 		struct crypto_tfm *tfm = *per_cpu_ptr(tfms, cpu);
-		if (tfm)
-			crypto_free_tfm(tfm);
+		crypto_free_tfm(tfm);
 	}
 	free_percpu(tfms);
 }
--- linux-2.6.12-rc3-mm1-orig/net/ipv6/addrconf.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/addrconf.c	2005-04-30 19:59:34.000000000 +0200
@@ -2966,8 +2966,7 @@ static int inet6_fill_ifinfo(struct sk_b
 
 nlmsg_failure:
 rtattr_failure:
-	if (array)
-		kfree(array);
+	kfree(array);
 	skb_trim(skb, b - skb->data);
 	return -1;
 }
@@ -3604,10 +3603,8 @@ void __exit addrconf_cleanup(void)
 	rtnl_unlock();
 
 #ifdef CONFIG_IPV6_PRIVACY
-	if (likely(md5_tfm != NULL)) {
-		crypto_free_tfm(md5_tfm);
-		md5_tfm = NULL;
-	}
+	crypto_free_tfm(md5_tfm);
+	md5_tfm = NULL;
 #endif
 
 #ifdef CONFIG_PROC_FS
--- linux-2.6.12-rc3-mm1-orig/net/ipv6/ah6.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/ah6.c	2005-04-30 20:01:39.000000000 +0200
@@ -217,12 +217,11 @@ static int ah6_output(struct xfrm_state 
 	err = 0;
 
 	memcpy(top_iph, tmp_base, sizeof(tmp_base));
-	if (tmp_ext) {
+	if (tmp_ext)
 		memcpy(&top_iph->daddr, tmp_ext, extlen);
-error_free_iph:
-		kfree(tmp_ext);
-	}
 
+error_free_iph:
+	kfree(tmp_ext);
 error:
 	return err;
 }
@@ -306,7 +305,6 @@ static int ah6_input(struct xfrm_state *
 	skb_pull(skb, hdr_len);
 	skb->h.raw = skb->data;
 
-
 	kfree(tmp_hdr);
 
 	return nexthdr;
@@ -402,10 +400,8 @@ static int ah6_init_state(struct xfrm_st
 
 error:
 	if (ahp) {
-		if (ahp->work_icv)
-			kfree(ahp->work_icv);
-		if (ahp->tfm)
-			crypto_free_tfm(ahp->tfm);
+		kfree(ahp->work_icv);
+		crypto_free_tfm(ahp->tfm);
 		kfree(ahp);
 	}
 	return -EINVAL;
@@ -418,14 +414,12 @@ static void ah6_destroy(struct xfrm_stat
 	if (!ahp)
 		return;
 
-	if (ahp->work_icv) {
-		kfree(ahp->work_icv);
-		ahp->work_icv = NULL;
-	}
-	if (ahp->tfm) {
-		crypto_free_tfm(ahp->tfm);
-		ahp->tfm = NULL;
-	}
+	kfree(ahp->work_icv);
+	ahp->work_icv = NULL;
+
+	crypto_free_tfm(ahp->tfm);
+	ahp->tfm = NULL;
+
 	kfree(ahp);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/ipv6/esp6.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/esp6.c	2005-04-30 19:41:06.000000000 +0200
@@ -277,22 +277,18 @@ static void esp6_destroy(struct xfrm_sta
 	if (!esp)
 		return;
 
-	if (esp->conf.tfm) {
-		crypto_free_tfm(esp->conf.tfm);
-		esp->conf.tfm = NULL;
-	}
-	if (esp->conf.ivec) {
-		kfree(esp->conf.ivec);
-		esp->conf.ivec = NULL;
-	}
-	if (esp->auth.tfm) {
-		crypto_free_tfm(esp->auth.tfm);
-		esp->auth.tfm = NULL;
-	}
-	if (esp->auth.work_icv) {
-		kfree(esp->auth.work_icv);
-		esp->auth.work_icv = NULL;
-	}
+	crypto_free_tfm(esp->conf.tfm);
+	esp->conf.tfm = NULL;
+
+	kfree(esp->conf.ivec);
+	esp->conf.ivec = NULL;
+
+	crypto_free_tfm(esp->auth.tfm);
+	esp->auth.tfm = NULL;
+
+	kfree(esp->auth.work_icv);
+	esp->auth.work_icv = NULL;
+
 	kfree(esp);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/ipv6/ipcomp6.c	2005-04-30 18:25:33.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/ipcomp6.c	2005-04-30 20:05:02.000000000 +0200
@@ -130,8 +130,7 @@ static int ipcomp6_input(struct xfrm_sta
 out_put_cpu:
 	put_cpu();
 out:
-	if (tmp_hdr)
-		kfree(tmp_hdr);
+	kfree(tmp_hdr);
 	if (err)
 		goto error_out;
 	return nexthdr;
@@ -292,8 +291,7 @@ static void ipcomp6_free_scratches(void)
 
 	for_each_cpu(i) {
 		void *scratch = *per_cpu_ptr(scratches, i);
-		if (scratch)
-			vfree(scratch);
+		vfree(scratch);
 	}
 
 	free_percpu(scratches);
@@ -346,8 +344,7 @@ static void ipcomp6_free_tfms(struct cry
 
 	for_each_cpu(cpu) {
 		struct crypto_tfm *tfm = *per_cpu_ptr(tfms, cpu);
-		if (tfm)
-			crypto_free_tfm(tfm);
+		crypto_free_tfm(tfm);
 	}
 	free_percpu(tfms);
 }
--- linux-2.6.12-rc3-mm1-orig/net/sctp/endpointola.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sctp/endpointola.c	2005-04-30 19:41:06.000000000 +0200
@@ -195,8 +195,7 @@ static void sctp_endpoint_destroy(struct
 	sctp_unhash_endpoint(ep);
 
 	/* Free up the HMAC transform. */
-	if (sctp_sk(ep->base.sk)->hmac)
-		sctp_crypto_free_tfm(sctp_sk(ep->base.sk)->hmac);
+	sctp_crypto_free_tfm(sctp_sk(ep->base.sk)->hmac);
 
 	/* Cleanup. */
 	sctp_inq_free(&ep->base.inqueue);
--- linux-2.6.12-rc3-mm1-orig/net/sctp/socket.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sctp/socket.c	2005-04-30 19:41:06.000000000 +0200
@@ -4022,8 +4022,7 @@ out:
 	sctp_release_sock(sk);
 	return err;
 cleanup:
-	if (tfm)
-		sctp_crypto_free_tfm(tfm);
+	sctp_crypto_free_tfm(tfm);
 	goto out;
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/sunrpc/auth_gss/gss_krb5_crypto.c	2005-03-02 08:37:47.000000000 +0100
+++ linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_krb5_crypto.c	2005-04-30 19:41:06.000000000 +0200
@@ -201,8 +201,7 @@ make_checksum(s32 cksumtype, char *heade
 	crypto_digest_final(tfm, cksum->data);
 	code = 0;
 out:
-	if (tfm)
-		crypto_free_tfm(tfm);
+	crypto_free_tfm(tfm);
 	return code;
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/sunrpc/auth_gss/gss_krb5_mech.c	2005-04-30 18:25:34.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_krb5_mech.c	2005-04-30 19:41:06.000000000 +0200
@@ -185,12 +185,9 @@ static void
 gss_delete_sec_context_kerberos(void *internal_ctx) {
 	struct krb5_ctx *kctx = internal_ctx;
 
-	if (kctx->seq)
-		crypto_free_tfm(kctx->seq);
-	if (kctx->enc)
-		crypto_free_tfm(kctx->enc);
-	if (kctx->mech_used.data)
-		kfree(kctx->mech_used.data);
+	crypto_free_tfm(kctx->seq);
+	crypto_free_tfm(kctx->enc);
+	kfree(kctx->mech_used.data);
 	kfree(kctx);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/sunrpc/auth_gss/gss_spkm3_mech.c	2005-04-30 18:25:34.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_spkm3_mech.c	2005-04-30 20:03:28.000000000 +0200
@@ -121,14 +121,14 @@ get_key(const void *p, const void *end, 
 			goto out_err_free_tfm;
 	}
 
-	if(key.len > 0)
+	if (key.len > 0)
 		kfree(key.data);
 	return p;
 
 out_err_free_tfm:
 	crypto_free_tfm(*res);
 out_err_free_key:
-	if(key.len > 0)
+	if (key.len > 0)
 		kfree(key.data);
 	p = ERR_PTR(-EINVAL);
 out_err:
@@ -214,14 +214,10 @@ static void
 gss_delete_sec_context_spkm3(void *internal_ctx) {
 	struct spkm3_ctx *sctx = internal_ctx;
 
-	if(sctx->derived_integ_key)
-		crypto_free_tfm(sctx->derived_integ_key);
-	if(sctx->derived_conf_key)
-		crypto_free_tfm(sctx->derived_conf_key);
-	if(sctx->share_key.data)
-		kfree(sctx->share_key.data);
-	if(sctx->mech_used.data)
-		kfree(sctx->mech_used.data);
+	crypto_free_tfm(sctx->derived_integ_key);
+	crypto_free_tfm(sctx->derived_conf_key);
+	kfree(sctx->share_key.data);
+	kfree(sctx->mech_used.data);
 	kfree(sctx);
 }
 

Re: [PATCH] resource release cleanup in net/

[Arnaldo Carvalho de Melo]

On 4/30/05, Jesper Juhl <juhl-lkml@dif.dk> wrote:
> Hi David,
> 
> Since Andrew merged the patch that makes calling crypto_free_tfm() with a
> NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for
> NULL before calling that function, and while I was at it I removed similar
> redundant checks before calls to kfree() and vfree() in the same files.
> There are also a few tiny whitespace cleanups in there.

Jesper, I'd suggest that you left whitespaces for a separate patch, it
is always,
IMHO, better to have as small a patch as possible for reviewing.

- Arnaldo

Re: [PATCH] resource release cleanup in net/

[Jesper Juhl]

On Sun, 1 May 2005, Alexey Dobriyan wrote:

> On Sat, Apr 30, 2005 at 10:36:00PM +0200, Jesper Juhl wrote:
> > Since Andrew merged the patch that makes calling crypto_free_tfm() with a 
> > NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for 
> > NULL before calling that function
> >  drivers/net/wireless/hostap/hostap_crypt_ccmp.c |    5 -
> >  drivers/net/wireless/hostap/hostap_crypt_tkip.c |   10 +-
> >  drivers/net/wireless/hostap/hostap_crypt_wep.c  |    5 -
> >  net/ieee80211/ieee80211_crypt_ccmp.c            |    5 -
> >  net/ieee80211/ieee80211_crypt_tkip.c            |   10 +-
> >  net/ieee80211/ieee80211_crypt_wep.c             |    5 -
> I think I have a better one for these.
> 
> --- linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-05-01 01:53:50.000000000 +0000
> +++ linux-2.6.12-rc3-mm1-hostap/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-05-01 02:21:10.000000000 +0000
> @@ -102,17 +102,14 @@ static void * hostap_ccmp_init(int key_i
>  	if (priv->tfm == NULL) {
>  		printk(KERN_DEBUG "hostap_crypt_ccmp: could not allocate "
>  		       "crypto API aes\n");
> -		goto fail;
> +		goto fail_free;
>  	}
>  
>  	return priv;
>  
> +fail_free:
> +	kfree(priv);
>  fail:
> -	if (priv) {
> -		if (priv->tfm)
> -			crypto_free_tfm(priv->tfm);
> -		kfree(priv);
> -	}
>  	module_put(THIS_MODULE);
>  	return NULL;
>  }

Personally I find the code a lot simpler when there's just a single label 
"fail" that takes care of all the cleanup. But, let's hear what David has 
to say on that.


> @@ -121,8 +118,7 @@ fail:
>  static void hostap_ccmp_deinit(void *priv)
>  {
>  	struct hostap_ccmp_data *_priv = priv;
> -	if (_priv && _priv->tfm)
> -		crypto_free_tfm(_priv->tfm);
> +	crypto_free_tfm(_priv->tfm);
>  	kfree(priv);
>  	module_put(THIS_MODULE);
>  }

This will Oops if _priv is NULL. That's why my patch did 

if (_priv)
	crypto_free_tfm(_priv->tfm);


> --- linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_tkip.c	2005-05-01 01:53:50.000000000 +0000
> +++ linux-2.6.12-rc3-mm1-hostap/drivers/net/wireless/hostap/hostap_crypt_tkip.c	2005-05-01 02:27:18.000000000 +0000
> @@ -88,26 +88,23 @@ static void * hostap_tkip_init(int key_i
>  	if (priv->tfm_arc4 == NULL) {
>  		printk(KERN_DEBUG "hostap_crypt_tkip: could not allocate "
>  		       "crypto API arc4\n");
> -		goto fail;
> +		goto fail_arc4;
>  	}
>  
>  	priv->tfm_michael = crypto_alloc_tfm("michael_mic", 0);
>  	if (priv->tfm_michael == NULL) {
>  		printk(KERN_DEBUG "hostap_crypt_tkip: could not allocate "
>  		       "crypto API michael_mic\n");
> -		goto fail;
> +		goto fail_michael;
>  	}
>  
>  	return priv;
>  
> +fail_michael:
> +	crypto_free_tfm(priv->tfm_arc4);
> +fail_arc4:
> +	kfree(priv);
>  fail:
> -	if (priv) {
> -		if (priv->tfm_michael)
> -			crypto_free_tfm(priv->tfm_michael);
> -		if (priv->tfm_arc4)
> -			crypto_free_tfm(priv->tfm_arc4);
> -		kfree(priv);
> -	}
>  	module_put(THIS_MODULE);
>  	return NULL;
>  }

Again, I find the single 'fail' approach simpler.


> @@ -116,10 +113,8 @@ fail:
>  static void hostap_tkip_deinit(void *priv)
>  {
>  	struct hostap_tkip_data *_priv = priv;
> -	if (_priv && _priv->tfm_michael)
> -		crypto_free_tfm(_priv->tfm_michael);
> -	if (_priv && _priv->tfm_arc4)
> -		crypto_free_tfm(_priv->tfm_arc4);
> +	crypto_free_tfm(_priv->tfm_michael);
> +	crypto_free_tfm(_priv->tfm_arc4);
>  	kfree(priv);
>  	module_put(THIS_MODULE);
>  }

Oops if _priv == NULL.


> --- linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_wep.c	2005-05-01 01:53:50.000000000 +0000
> +++ linux-2.6.12-rc3-mm1-hostap/drivers/net/wireless/hostap/hostap_crypt_wep.c	2005-05-01 02:30:08.000000000 +0000
> @@ -59,7 +59,7 @@ static void * prism2_wep_init(int keyidx
>  	if (priv->tfm == NULL) {
>  		printk(KERN_DEBUG "hostap_crypt_wep: could not allocate "
>  		       "crypto API arc4\n");
> -		goto fail;
> +		goto fail_tfm;
>  	}
>  
>  	/* start WEP IV from a random value */
> @@ -67,12 +67,9 @@ static void * prism2_wep_init(int keyidx
>  
>  	return priv;
>  
> +fail_tfm:
> +	kfree(priv);
>  fail:
> -	if (priv) {
> -		if (priv->tfm)
> -			crypto_free_tfm(priv->tfm);
> -		kfree(priv);
> -	}
>  	module_put(THIS_MODULE);
>  	return NULL;
>  }

Again my preference is for the original, same for the rest of these.


> @@ -81,8 +78,7 @@ fail:
>  static void prism2_wep_deinit(void *priv)
>  {
>  	struct prism2_wep_data *_priv = priv;
> -	if (_priv && _priv->tfm)
> -		crypto_free_tfm(_priv->tfm);
> +	crypto_free_tfm(_priv->tfm);
>  	kfree(priv);
>  	module_put(THIS_MODULE);
>  }

Oops if _priv == NULL.


> --- linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_ccmp.c	2005-05-01 01:53:57.000000000 +0000
> +++ linux-2.6.12-rc3-mm1-hostap/net/ieee80211/ieee80211_crypt_ccmp.c	2005-05-01 02:31:22.000000000 +0000
> @@ -89,18 +89,14 @@ static void * ieee80211_ccmp_init(int ke
>  	if (priv->tfm == NULL) {
>  		printk(KERN_DEBUG "ieee80211_crypt_ccmp: could not allocate "
>  		       "crypto API aes\n");
> -		goto fail;
> +		goto fail_tfm;
>  	}
>  
>  	return priv;
>  
> +fail_tfm:
> +	kfree(priv);
>  fail:
> -	if (priv) {
> -		if (priv->tfm)
> -			crypto_free_tfm(priv->tfm);
> -		kfree(priv);
> -	}
> -
>  	return NULL;
>  }
>  
> @@ -108,8 +104,7 @@ fail:
>  static void ieee80211_ccmp_deinit(void *priv)
>  {
>  	struct ieee80211_ccmp_data *_priv = priv;
> -	if (_priv && _priv->tfm)
> -		crypto_free_tfm(_priv->tfm);
> +	crypto_free_tfm(_priv->tfm);
>  	kfree(priv);
>  }
>  
if _priv is NULL this blows up.


> --- linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_tkip.c	2005-05-01 01:53:57.000000000 +0000
> +++ linux-2.6.12-rc3-mm1-hostap/net/ieee80211/ieee80211_crypt_tkip.c	2005-05-01 02:34:04.000000000 +0000
> @@ -76,27 +76,23 @@ static void * ieee80211_tkip_init(int ke
>  	if (priv->tfm_arc4 == NULL) {
>  		printk(KERN_DEBUG "ieee80211_crypt_tkip: could not allocate "
>  		       "crypto API arc4\n");
> -		goto fail;
> +		goto fail_arc4;
>  	}
>  
>  	priv->tfm_michael = crypto_alloc_tfm("michael_mic", 0);
>  	if (priv->tfm_michael == NULL) {
>  		printk(KERN_DEBUG "ieee80211_crypt_tkip: could not allocate "
>  		       "crypto API michael_mic\n");
> -		goto fail;
> +		goto fail_michael;
>  	}
>  
>  	return priv;
>  
> +fail_michael:
> +	crypto_free_tfm(priv->tfm_arc4);
> +fail_arc4:
> +	kfree(priv);
>  fail:
> -	if (priv) {
> -		if (priv->tfm_michael)
> -			crypto_free_tfm(priv->tfm_michael);
> -		if (priv->tfm_arc4)
> -			crypto_free_tfm(priv->tfm_arc4);
> -		kfree(priv);
> -	}
> -
>  	return NULL;
>  }
>  
> @@ -104,10 +100,8 @@ fail:
>  static void ieee80211_tkip_deinit(void *priv)
>  {
>  	struct ieee80211_tkip_data *_priv = priv;
> -	if (_priv && _priv->tfm_michael)
> -		crypto_free_tfm(_priv->tfm_michael);
> -	if (_priv && _priv->tfm_arc4)
> -		crypto_free_tfm(_priv->tfm_arc4);
> +	crypto_free_tfm(_priv->tfm_michael);
> +	crypto_free_tfm(_priv->tfm_arc4);
>  	kfree(priv);
>  }
>  
Again we'll blow up if _priv is NULL.


> --- linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_wep.c	2005-05-01 01:53:57.000000000 +0000
> +++ linux-2.6.12-rc3-mm1-hostap/net/ieee80211/ieee80211_crypt_wep.c	2005-05-01 02:35:18.000000000 +0000
> @@ -54,7 +54,7 @@ static void * prism2_wep_init(int keyidx
>  	if (priv->tfm == NULL) {
>  		printk(KERN_DEBUG "ieee80211_crypt_wep: could not allocate "
>  		       "crypto API arc4\n");
> -		goto fail;
> +		goto fail_tfm;
>  	}
>  
>  	/* start WEP IV from a random value */
> @@ -62,12 +62,9 @@ static void * prism2_wep_init(int keyidx
>  
>  	return priv;
>  
> +fail_tfm:
> +	kfree(priv);
>  fail:
> -	if (priv) {
> -		if (priv->tfm)
> -			crypto_free_tfm(priv->tfm);
> -		kfree(priv);
> -	}
>  	return NULL;
>  }
>  
> @@ -75,8 +72,7 @@ fail:
>  static void prism2_wep_deinit(void *priv)
>  {
>  	struct prism2_wep_data *_priv = priv;
> -	if (_priv && _priv->tfm)
> -		crypto_free_tfm(_priv->tfm);
> +	crypto_free_tfm(_priv->tfm);
>  	kfree(priv);
>  }
>  
_priv == NULL == Oops.


I think my version of the patch is better.

Ohh and would you mind submitting patches inline? Makes it a lot easier to 
quote and comment.


-- 
Jesper 

[PATCH 0/4] resource release cleanup in net/ (take 2)

[Jesper Juhl]

On Sat, 30 Apr 2005, Arnaldo Carvalho de Melo wrote:

> On 4/30/05, Jesper Juhl <juhl-lkml@dif.dk> wrote:
> > Hi David,
> > 
> > Since Andrew merged the patch that makes calling crypto_free_tfm() with a
> > NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for
> > NULL before calling that function, and while I was at it I removed similar
> > redundant checks before calls to kfree() and vfree() in the same files.
> > There are also a few tiny whitespace cleanups in there.
> 
> Jesper, I'd suggest that you left whitespaces for a separate patch, it
> is always,
> IMHO, better to have as small a patch as possible for reviewing.
> 
Sure thing. I've split the patches, and I believe that me going through 
them a second time did them good, there are a few tiny changes over the 
first version.

I split the patch in 4 parts (will send as replies to this mail) : 
	1) crypto_free_tfm related changes
	2) kfree related changes
	3) vfree related changes
	4) whitespace changes
The whitespace changes ended up fairly bigger than initially. I expanded 
the cleanup a bit. It's not a perfect, 100% complete cleanup, but it's IMO 
a lot better than the originals.

-- 
Jesper

[PATCH 1/4] resource release cleanup in net/ (take 2)

[Jesper Juhl]

On Sun, 1 May 2005, Jesper Juhl wrote:

> On Sat, 30 Apr 2005, Arnaldo Carvalho de Melo wrote:
> 
> > On 4/30/05, Jesper Juhl <juhl-lkml@dif.dk> wrote:
> > > Hi David,
> > > 
> > > Since Andrew merged the patch that makes calling crypto_free_tfm() with a
> > > NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for
> > > NULL before calling that function, and while I was at it I removed similar
> > > redundant checks before calls to kfree() and vfree() in the same files.
> > > There are also a few tiny whitespace cleanups in there.
> > 
> > Jesper, I'd suggest that you left whitespaces for a separate patch, it
> > is always,
> > IMHO, better to have as small a patch as possible for reviewing.
> > 
> Sure thing. I've split the patches, and I believe that me going through 
> them a second time did them good, there are a few tiny changes over the 
> first version.
> 
> I split the patch in 4 parts (will send as replies to this mail) : 
> 	1) crypto_free_tfm related changes
> 	2) kfree related changes
> 	3) vfree related changes
> 	4) whitespace changes
> The whitespace changes ended up fairly bigger than initially. I expanded 
> the cleanup a bit. It's not a perfect, 100% complete cleanup, but it's IMO 
> a lot better than the originals.
> 

Here are the crypto_free_tfm changes.

Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
---

 drivers/net/wireless/airo.c                     |    3 +--
 drivers/net/wireless/hostap/hostap_crypt_ccmp.c |    5 ++---
 drivers/net/wireless/hostap/hostap_crypt_tkip.c |   10 ++++------
 drivers/net/wireless/hostap/hostap_crypt_wep.c  |    5 ++---
 net/ieee80211/ieee80211_crypt_ccmp.c            |    5 ++---
 net/ieee80211/ieee80211_crypt_tkip.c            |   10 ++++------
 net/ieee80211/ieee80211_crypt_wep.c             |    5 ++---
 net/ipv4/ah4.c                                  |    9 +++------
 net/ipv4/esp4.c                                 |   12 ++++--------
 net/ipv4/ipcomp.c                               |    3 +--
 net/ipv6/addrconf.c                             |    6 ++----
 net/ipv6/ah6.c                                  |    9 +++------
 net/ipv6/esp6.c                                 |   12 ++++--------
 net/ipv6/ipcomp6.c                              |    3 +--
 net/sctp/endpointola.c                          |    3 +--
 net/sctp/socket.c                               |    3 +--
 net/sunrpc/auth_gss/gss_krb5_crypto.c           |    3 +--
 net/sunrpc/auth_gss/gss_krb5_mech.c             |    6 ++----
 net/sunrpc/auth_gss/gss_spkm3_mech.c            |    6 ++----
 19 files changed, 42 insertions(+), 76 deletions(-)

--- linux-2.6.12-rc3-mm1-orig/drivers/net/wireless/airo.c	2005-04-30 18:37:01.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/airo.c	2005-05-01 01:43:45.000000000 +0200
@@ -2406,8 +2406,7 @@
 		}
         }
 #ifdef MICSUPPORT
-	if (ai->tfm)
-		crypto_free_tfm(ai->tfm);
+	crypto_free_tfm(ai->tfm);
 #endif
 	del_airo_dev( dev );
 	free_netdev( dev );
--- linux-2.6.12-rc3-mm1-orig/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-04-30 18:37:01.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-05-01 01:44:07.000000000 +0200
@@ -109,8 +109,7 @@
 
 fail:
 	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
+		crypto_free_tfm(priv->tfm);
 		kfree(priv);
 	}
 	module_put(THIS_MODULE);
@@ -121,7 +120,7 @@
 static void hostap_ccmp_deinit(void *priv)
 {
 	struct hostap_ccmp_data *_priv = priv;
-	if (_priv && _priv->tfm)
+	if (_priv)
 		crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 	module_put(THIS_MODULE);
--- linux-2.6.12-rc3-mm1-orig/drivers/net/wireless/hostap/hostap_crypt_tkip.c	2005-04-30 18:37:01.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_tkip.c	2005-05-01 01:44:54.000000000 +0200
@@ -102,10 +102,8 @@
 
 fail:
 	if (priv) {
-		if (priv->tfm_michael)
-			crypto_free_tfm(priv->tfm_michael);
-		if (priv->tfm_arc4)
-			crypto_free_tfm(priv->tfm_arc4);
+		crypto_free_tfm(priv->tfm_michael);
+		crypto_free_tfm(priv->tfm_arc4);
 		kfree(priv);
 	}
 	module_put(THIS_MODULE);
@@ -116,10 +114,10 @@
 static void hostap_tkip_deinit(void *priv)
 {
 	struct hostap_tkip_data *_priv = priv;
-	if (_priv && _priv->tfm_michael)
+	if (_priv) {
 		crypto_free_tfm(_priv->tfm_michael);
-	if (_priv && _priv->tfm_arc4)
 		crypto_free_tfm(_priv->tfm_arc4);
+	}
 	kfree(priv);
 	module_put(THIS_MODULE);
 }
--- linux-2.6.12-rc3-mm1-orig/drivers/net/wireless/hostap/hostap_crypt_wep.c	2005-04-30 18:37:01.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_wep.c	2005-05-01 01:45:11.000000000 +0200
@@ -69,8 +69,7 @@
 
 fail:
 	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
+		crypto_free_tfm(priv->tfm);
 		kfree(priv);
 	}
 	module_put(THIS_MODULE);
@@ -81,7 +80,7 @@
 static void prism2_wep_deinit(void *priv)
 {
 	struct prism2_wep_data *_priv = priv;
-	if (_priv && _priv->tfm)
+	if (_priv)
 		crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 	module_put(THIS_MODULE);
--- linux-2.6.12-rc3-mm1-orig/net/ieee80211/ieee80211_crypt_ccmp.c	2005-04-30 18:37:16.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_ccmp.c	2005-05-01 01:45:28.000000000 +0200
@@ -96,8 +96,7 @@
 
 fail:
 	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
+		crypto_free_tfm(priv->tfm);
 		kfree(priv);
 	}
 
@@ -108,7 +107,7 @@
 static void ieee80211_ccmp_deinit(void *priv)
 {
 	struct ieee80211_ccmp_data *_priv = priv;
-	if (_priv && _priv->tfm)
+	if (_priv)
 		crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 }
--- linux-2.6.12-rc3-mm1-orig/net/ieee80211/ieee80211_crypt_tkip.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_tkip.c	2005-05-01 01:47:04.000000000 +0200
@@ -90,10 +90,8 @@
 
 fail:
 	if (priv) {
-		if (priv->tfm_michael)
-			crypto_free_tfm(priv->tfm_michael);
-		if (priv->tfm_arc4)
-			crypto_free_tfm(priv->tfm_arc4);
+		crypto_free_tfm(priv->tfm_michael);
+		crypto_free_tfm(priv->tfm_arc4);
 		kfree(priv);
 	}
 
@@ -104,10 +102,10 @@
 static void ieee80211_tkip_deinit(void *priv)
 {
 	struct ieee80211_tkip_data *_priv = priv;
-	if (_priv && _priv->tfm_michael)
+	if (_priv) {
 		crypto_free_tfm(_priv->tfm_michael);
-	if (_priv && _priv->tfm_arc4)
 		crypto_free_tfm(_priv->tfm_arc4);
+	}
 	kfree(priv);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/ieee80211/ieee80211_crypt_wep.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_wep.c	2005-05-01 01:47:24.000000000 +0200
@@ -64,8 +64,7 @@
 
 fail:
 	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
+		crypto_free_tfm(priv->tfm);
 		kfree(priv);
 	}
 	return NULL;
@@ -75,7 +74,7 @@
 static void prism2_wep_deinit(void *priv)
 {
 	struct prism2_wep_data *_priv = priv;
-	if (_priv && _priv->tfm)
+	if (_priv)
 		crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 }
--- linux-2.6.12-rc3-mm1-orig/net/ipv4/ah4.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/ah4.c	2005-05-01 01:48:11.000000000 +0200
@@ -265,8 +265,7 @@
 	if (ahp) {
 		if (ahp->work_icv)
 			kfree(ahp->work_icv);
-		if (ahp->tfm)
-			crypto_free_tfm(ahp->tfm);
+		crypto_free_tfm(ahp->tfm);
 		kfree(ahp);
 	}
 	return -EINVAL;
@@ -283,10 +282,8 @@
 		kfree(ahp->work_icv);
 		ahp->work_icv = NULL;
 	}
-	if (ahp->tfm) {
-		crypto_free_tfm(ahp->tfm);
-		ahp->tfm = NULL;
-	}
+	crypto_free_tfm(ahp->tfm);
+	ahp->tfm = NULL;
 	kfree(ahp);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/ipv4/esp4.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/esp4.c	2005-05-01 01:48:36.000000000 +0200
@@ -343,18 +343,14 @@
 	if (!esp)
 		return;
 
-	if (esp->conf.tfm) {
-		crypto_free_tfm(esp->conf.tfm);
-		esp->conf.tfm = NULL;
-	}
+	crypto_free_tfm(esp->conf.tfm);
+	esp->conf.tfm = NULL;
 	if (esp->conf.ivec) {
 		kfree(esp->conf.ivec);
 		esp->conf.ivec = NULL;
 	}
-	if (esp->auth.tfm) {
-		crypto_free_tfm(esp->auth.tfm);
-		esp->auth.tfm = NULL;
-	}
+	crypto_free_tfm(esp->auth.tfm);
+	esp->auth.tfm = NULL;
 	if (esp->auth.work_icv) {
 		kfree(esp->auth.work_icv);
 		esp->auth.work_icv = NULL;
--- linux-2.6.12-rc3-mm1-orig/net/ipv4/ipcomp.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/ipcomp.c	2005-05-01 01:49:25.000000000 +0200
@@ -350,8 +350,7 @@
 
 	for_each_cpu(cpu) {
 		struct crypto_tfm *tfm = *per_cpu_ptr(tfms, cpu);
-		if (tfm)
-			crypto_free_tfm(tfm);
+		crypto_free_tfm(tfm);
 	}
 	free_percpu(tfms);
 }
--- linux-2.6.12-rc3-mm1-orig/net/ipv6/addrconf.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/addrconf.c	2005-05-01 01:50:00.000000000 +0200
@@ -3604,10 +3604,8 @@
 	rtnl_unlock();
 
 #ifdef CONFIG_IPV6_PRIVACY
-	if (likely(md5_tfm != NULL)) {
-		crypto_free_tfm(md5_tfm);
-		md5_tfm = NULL;
-	}
+	crypto_free_tfm(md5_tfm);
+	md5_tfm = NULL;
 #endif
 
 #ifdef CONFIG_PROC_FS
--- linux-2.6.12-rc3-mm1-orig/net/ipv6/ah6.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/ah6.c	2005-05-01 01:50:22.000000000 +0200
@@ -404,8 +404,7 @@
 	if (ahp) {
 		if (ahp->work_icv)
 			kfree(ahp->work_icv);
-		if (ahp->tfm)
-			crypto_free_tfm(ahp->tfm);
+		crypto_free_tfm(ahp->tfm);
 		kfree(ahp);
 	}
 	return -EINVAL;
@@ -422,10 +421,8 @@
 		kfree(ahp->work_icv);
 		ahp->work_icv = NULL;
 	}
-	if (ahp->tfm) {
-		crypto_free_tfm(ahp->tfm);
-		ahp->tfm = NULL;
-	}
+	crypto_free_tfm(ahp->tfm);
+	ahp->tfm = NULL;
 	kfree(ahp);
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/ipv6/esp6.c	2005-04-30 18:25:32.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/esp6.c	2005-05-01 01:50:37.000000000 +0200
@@ -277,18 +277,14 @@
 	if (!esp)
 		return;
 
-	if (esp->conf.tfm) {
-		crypto_free_tfm(esp->conf.tfm);
-		esp->conf.tfm = NULL;
-	}
+	crypto_free_tfm(esp->conf.tfm);
+	esp->conf.tfm = NULL;
 	if (esp->conf.ivec) {
 		kfree(esp->conf.ivec);
 		esp->conf.ivec = NULL;
 	}
-	if (esp->auth.tfm) {
-		crypto_free_tfm(esp->auth.tfm);
-		esp->auth.tfm = NULL;
-	}
+	crypto_free_tfm(esp->auth.tfm);
+	esp->auth.tfm = NULL;
 	if (esp->auth.work_icv) {
 		kfree(esp->auth.work_icv);
 		esp->auth.work_icv = NULL;
--- linux-2.6.12-rc3-mm1-orig/net/ipv6/ipcomp6.c	2005-04-30 18:25:33.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/ipcomp6.c	2005-05-01 01:50:46.000000000 +0200
@@ -346,8 +346,7 @@
 
 	for_each_cpu(cpu) {
 		struct crypto_tfm *tfm = *per_cpu_ptr(tfms, cpu);
-		if (tfm)
-			crypto_free_tfm(tfm);
+		crypto_free_tfm(tfm);
 	}
 	free_percpu(tfms);
 }
--- linux-2.6.12-rc3-mm1-orig/net/sctp/endpointola.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sctp/endpointola.c	2005-05-01 01:51:03.000000000 +0200
@@ -195,8 +195,7 @@
 	sctp_unhash_endpoint(ep);
 
 	/* Free up the HMAC transform. */
-	if (sctp_sk(ep->base.sk)->hmac)
-		sctp_crypto_free_tfm(sctp_sk(ep->base.sk)->hmac);
+	sctp_crypto_free_tfm(sctp_sk(ep->base.sk)->hmac);
 
 	/* Cleanup. */
 	sctp_inq_free(&ep->base.inqueue);
--- linux-2.6.12-rc3-mm1-orig/net/sctp/socket.c	2005-04-30 18:37:17.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sctp/socket.c	2005-05-01 01:51:11.000000000 +0200
@@ -4022,8 +4022,7 @@
 	sctp_release_sock(sk);
 	return err;
 cleanup:
-	if (tfm)
-		sctp_crypto_free_tfm(tfm);
+	sctp_crypto_free_tfm(tfm);
 	goto out;
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/sunrpc/auth_gss/gss_krb5_crypto.c	2005-03-02 08:37:47.000000000 +0100
+++ linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_krb5_crypto.c	2005-05-01 01:51:19.000000000 +0200
@@ -201,8 +201,7 @@
 	crypto_digest_final(tfm, cksum->data);
 	code = 0;
 out:
-	if (tfm)
-		crypto_free_tfm(tfm);
+	crypto_free_tfm(tfm);
 	return code;
 }
 
--- linux-2.6.12-rc3-mm1-orig/net/sunrpc/auth_gss/gss_krb5_mech.c	2005-04-30 18:25:34.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_krb5_mech.c	2005-05-01 01:51:41.000000000 +0200
@@ -185,10 +185,8 @@
 gss_delete_sec_context_kerberos(void *internal_ctx) {
 	struct krb5_ctx *kctx = internal_ctx;
 
-	if (kctx->seq)
-		crypto_free_tfm(kctx->seq);
-	if (kctx->enc)
-		crypto_free_tfm(kctx->enc);
+	crypto_free_tfm(kctx->seq);
+	crypto_free_tfm(kctx->enc);
 	if (kctx->mech_used.data)
 		kfree(kctx->mech_used.data);
 	kfree(kctx);
--- linux-2.6.12-rc3-mm1-orig/net/sunrpc/auth_gss/gss_spkm3_mech.c	2005-04-30 18:25:34.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_spkm3_mech.c	2005-05-01 01:52:04.000000000 +0200
@@ -214,10 +214,8 @@
 gss_delete_sec_context_spkm3(void *internal_ctx) {
 	struct spkm3_ctx *sctx = internal_ctx;
 
-	if(sctx->derived_integ_key)
-		crypto_free_tfm(sctx->derived_integ_key);
-	if(sctx->derived_conf_key)
-		crypto_free_tfm(sctx->derived_conf_key);
+	crypto_free_tfm(sctx->derived_integ_key);
+	crypto_free_tfm(sctx->derived_conf_key);
 	if(sctx->share_key.data)
 		kfree(sctx->share_key.data);
 	if(sctx->mech_used.data)

Re: [PATCH 2/4] resource release cleanup in net/ (take 2)

[Jesper Juhl]

On Sun, 1 May 2005, Jesper Juhl wrote:

> On Sat, 30 Apr 2005, Arnaldo Carvalho de Melo wrote:
> 
> > On 4/30/05, Jesper Juhl <juhl-lkml@dif.dk> wrote:
> > > Hi David,
> > > 
> > > Since Andrew merged the patch that makes calling crypto_free_tfm() with a
> > > NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for
> > > NULL before calling that function, and while I was at it I removed similar
> > > redundant checks before calls to kfree() and vfree() in the same files.
> > > There are also a few tiny whitespace cleanups in there.
> > 
> > Jesper, I'd suggest that you left whitespaces for a separate patch, it
> > is always,
> > IMHO, better to have as small a patch as possible for reviewing.
> > 
> Sure thing. I've split the patches, and I believe that me going through 
> them a second time did them good, there are a few tiny changes over the 
> first version.
> 
> I split the patch in 4 parts (will send as replies to this mail) : 
> 	1) crypto_free_tfm related changes
> 	2) kfree related changes
> 	3) vfree related changes
> 	4) whitespace changes
> The whitespace changes ended up fairly bigger than initially. I expanded 
> the cleanup a bit. It's not a perfect, 100% complete cleanup, but it's IMO 
> a lot better than the originals.
> 

These are the kfree changes. Incremental patch over patch 1.

Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
---

 drivers/net/wireless/airo.c          |   41 ++++++++++++-----------------------
 net/ipv4/ah4.c                       |    9 ++-----
 net/ipv4/esp4.c                      |   12 +++-------
 net/ipv6/addrconf.c                  |    3 --
 net/ipv6/ah6.c                       |   16 +++++--------
 net/ipv6/esp6.c                      |   12 +++-------
 net/ipv6/ipcomp6.c                   |    3 --
 net/sunrpc/auth_gss/gss_krb5_mech.c  |    3 --
 net/sunrpc/auth_gss/gss_spkm3_mech.c |    6 +----
 9 files changed, 37 insertions(+), 68 deletions(-)

--- linux-2.6.12-rc3-mm1/drivers/net/wireless/airo.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/drivers/net/wireless/airo.c	2005-05-01 03:23:56.000000000 +0200
@@ -2383,14 +2383,10 @@
 			dev_kfree_skb(skb);
 	}
 
-	if (ai->flash)
-		kfree(ai->flash);
-	if (ai->rssi)
-		kfree(ai->rssi);
-	if (ai->APList)
-		kfree(ai->APList);
-	if (ai->SSID)
-		kfree(ai->SSID);
+	kfree(ai->flash);
+	kfree(ai->rssi);
+	kfree(ai->APList);
+	kfree(ai->SSID);
 	if (freeres) {
 		/* PCMCIA frees this stuff, so only for PCI and ISA */
 	        release_region( dev->base_addr, 64 );
@@ -3627,10 +3623,8 @@
 	int rc;
 
 	memset( &mySsid, 0, sizeof( mySsid ) );
-	if (ai->flash) {
-		kfree (ai->flash);
-		ai->flash = NULL;
-	}
+	kfree(ai->flash);
+	ai->flash = NULL;
 
 	/* The NOP is the first step in getting the card going */
 	cmd.cmd = NOP;
@@ -3667,14 +3661,11 @@
 		tdsRssiRid rssi_rid;
 		CapabilityRid cap_rid;
 
-		if (ai->APList) {
-			kfree(ai->APList);
-			ai->APList = NULL;
-		}
-		if (ai->SSID) {
-			kfree(ai->SSID);
-			ai->SSID = NULL;
-		}
+		kfree(ai->APList);
+		ai->APList = NULL;
+		kfree(ai->SSID);
+		ai->SSID = NULL;
+
 		// general configuration (read/modify/write)
 		status = readConfigRid(ai, lock);
 		if ( status != SUCCESS ) return ERROR;
@@ -3688,10 +3679,8 @@
 				memcpy(ai->rssi, (u8*)&rssi_rid + 2, 512);
 		}
 		else {
-			if (ai->rssi) {
-				kfree(ai->rssi);
-				ai->rssi = NULL;
-			}
+			kfree(ai->rssi);
+			ai->rssi = NULL;
 			if (cap_rid.softCap & 8)
 				ai->config.rmode |= RXMODE_NORMALIZED_RSSI;
 			else
@@ -5372,8 +5361,8 @@
 {
 	struct proc_data *data = (struct proc_data *)file->private_data;
 	if ( data->on_close != NULL ) data->on_close( inode, file );
-	if ( data->rbuffer ) kfree( data->rbuffer );
-	if ( data->wbuffer ) kfree( data->wbuffer );
+	kfree(data->rbuffer);
+	kfree(data->wbuffer);
 	kfree( data );
 	return 0;
 }
--- linux-2.6.12-rc3-mm1/net/ipv4/ah4.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/ah4.c	2005-05-01 03:23:56.000000000 +0200
@@ -263,8 +263,7 @@
 
 error:
 	if (ahp) {
-		if (ahp->work_icv)
-			kfree(ahp->work_icv);
+		kfree(ahp->work_icv);
 		crypto_free_tfm(ahp->tfm);
 		kfree(ahp);
 	}
@@ -278,10 +277,8 @@
 	if (!ahp)
 		return;
 
-	if (ahp->work_icv) {
-		kfree(ahp->work_icv);
-		ahp->work_icv = NULL;
-	}
+	kfree(ahp->work_icv);
+	ahp->work_icv = NULL;
 	crypto_free_tfm(ahp->tfm);
 	ahp->tfm = NULL;
 	kfree(ahp);
--- linux-2.6.12-rc3-mm1/net/ipv4/esp4.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/esp4.c	2005-05-01 03:23:56.000000000 +0200
@@ -345,16 +345,12 @@
 
 	crypto_free_tfm(esp->conf.tfm);
 	esp->conf.tfm = NULL;
-	if (esp->conf.ivec) {
-		kfree(esp->conf.ivec);
-		esp->conf.ivec = NULL;
-	}
+	kfree(esp->conf.ivec);
+	esp->conf.ivec = NULL;
 	crypto_free_tfm(esp->auth.tfm);
 	esp->auth.tfm = NULL;
-	if (esp->auth.work_icv) {
-		kfree(esp->auth.work_icv);
-		esp->auth.work_icv = NULL;
-	}
+	kfree(esp->auth.work_icv);
+	esp->auth.work_icv = NULL;
 	kfree(esp);
 }
 
--- linux-2.6.12-rc3-mm1/net/ipv6/addrconf.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/addrconf.c	2005-05-01 03:23:56.000000000 +0200
@@ -2966,8 +2966,7 @@
 
 nlmsg_failure:
 rtattr_failure:
-	if (array)
-		kfree(array);
+	kfree(array);
 	skb_trim(skb, b - skb->data);
 	return -1;
 }
--- linux-2.6.12-rc3-mm1/net/ipv6/ah6.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/ah6.c	2005-05-01 03:23:56.000000000 +0200
@@ -217,12 +217,11 @@
 	err = 0;
 
 	memcpy(top_iph, tmp_base, sizeof(tmp_base));
-	if (tmp_ext) {
+	if (tmp_ext)
 		memcpy(&top_iph->daddr, tmp_ext, extlen);
-error_free_iph:
-		kfree(tmp_ext);
-	}
 
+error_free_iph:
+	kfree(tmp_ext);
 error:
 	return err;
 }
@@ -401,8 +400,7 @@
 
 error:
 	if (ahp) {
-		if (ahp->work_icv)
-			kfree(ahp->work_icv);
+		kfree(ahp->work_icv);
 		crypto_free_tfm(ahp->tfm);
 		kfree(ahp);
 	}
@@ -416,10 +414,8 @@
 	if (!ahp)
 		return;
 
-	if (ahp->work_icv) {
-		kfree(ahp->work_icv);
-		ahp->work_icv = NULL;
-	}
+	kfree(ahp->work_icv);
+	ahp->work_icv = NULL;
 	crypto_free_tfm(ahp->tfm);
 	ahp->tfm = NULL;
 	kfree(ahp);
--- linux-2.6.12-rc3-mm1/net/ipv6/esp6.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/esp6.c	2005-05-01 03:23:56.000000000 +0200
@@ -279,16 +279,12 @@
 
 	crypto_free_tfm(esp->conf.tfm);
 	esp->conf.tfm = NULL;
-	if (esp->conf.ivec) {
-		kfree(esp->conf.ivec);
-		esp->conf.ivec = NULL;
-	}
+	kfree(esp->conf.ivec);
+	esp->conf.ivec = NULL;
 	crypto_free_tfm(esp->auth.tfm);
 	esp->auth.tfm = NULL;
-	if (esp->auth.work_icv) {
-		kfree(esp->auth.work_icv);
-		esp->auth.work_icv = NULL;
-	}
+	kfree(esp->auth.work_icv);
+	esp->auth.work_icv = NULL;
 	kfree(esp);
 }
 
--- linux-2.6.12-rc3-mm1/net/ipv6/ipcomp6.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/ipcomp6.c	2005-05-01 03:23:56.000000000 +0200
@@ -130,8 +130,7 @@
 out_put_cpu:
 	put_cpu();
 out:
-	if (tmp_hdr)
-		kfree(tmp_hdr);
+	kfree(tmp_hdr);
 	if (err)
 		goto error_out;
 	return nexthdr;
--- linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_krb5_mech.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_krb5_mech.c	2005-05-01 03:23:56.000000000 +0200
@@ -187,8 +187,7 @@
 
 	crypto_free_tfm(kctx->seq);
 	crypto_free_tfm(kctx->enc);
-	if (kctx->mech_used.data)
-		kfree(kctx->mech_used.data);
+	kfree(kctx->mech_used.data);
 	kfree(kctx);
 }
 
--- linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_spkm3_mech.c.old1	2005-05-01 03:23:26.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/sunrpc/auth_gss/gss_spkm3_mech.c	2005-05-01 03:23:56.000000000 +0200
@@ -216,10 +216,8 @@
 
 	crypto_free_tfm(sctx->derived_integ_key);
 	crypto_free_tfm(sctx->derived_conf_key);
-	if(sctx->share_key.data)
-		kfree(sctx->share_key.data);
-	if(sctx->mech_used.data)
-		kfree(sctx->mech_used.data);
+	kfree(sctx->share_key.data);
+	kfree(sctx->mech_used.data);
 	kfree(sctx);
 }
 

[PATCH 3/4] resource release cleanup in net/ (take 2)

[Jesper Juhl]

On Sun, 1 May 2005, Jesper Juhl wrote:

> On Sat, 30 Apr 2005, Arnaldo Carvalho de Melo wrote:
> 
> > On 4/30/05, Jesper Juhl <juhl-lkml@dif.dk> wrote:
> > > Hi David,
> > > 
> > > Since Andrew merged the patch that makes calling crypto_free_tfm() with a
> > > NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for
> > > NULL before calling that function, and while I was at it I removed similar
> > > redundant checks before calls to kfree() and vfree() in the same files.
> > > There are also a few tiny whitespace cleanups in there.
> > 
> > Jesper, I'd suggest that you left whitespaces for a separate patch, it
> > is always,
> > IMHO, better to have as small a patch as possible for reviewing.
> > 
> Sure thing. I've split the patches, and I believe that me going through 
> them a second time did them good, there are a few tiny changes over the 
> first version.
> 
> I split the patch in 4 parts (will send as replies to this mail) : 
> 	1) crypto_free_tfm related changes
> 	2) kfree related changes
> 	3) vfree related changes
> 	4) whitespace changes
> The whitespace changes ended up fairly bigger than initially. I expanded 
> the cleanup a bit. It's not a perfect, 100% complete cleanup, but it's IMO 
> a lot better than the originals.
> 

These are the vfree changes. Incremental patch on top of patch 2.


Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
---

 net/ipv4/ipcomp.c  |    3 +--
 net/ipv6/ipcomp6.c |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

--- linux-2.6.12-rc3-mm1/net/ipv4/ipcomp.c.old2	2005-05-01 03:23:56.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv4/ipcomp.c	2005-05-01 03:24:09.000000000 +0200
@@ -296,8 +296,7 @@
 
 	for_each_cpu(i) {
 		void *scratch = *per_cpu_ptr(scratches, i);
-		if (scratch)
-			vfree(scratch);
+		vfree(scratch);
 	}
 
 	free_percpu(scratches);
--- linux-2.6.12-rc3-mm1/net/ipv6/ipcomp6.c.old2	2005-05-01 03:23:56.000000000 +0200
+++ linux-2.6.12-rc3-mm1/net/ipv6/ipcomp6.c	2005-05-01 03:24:09.000000000 +0200
@@ -291,8 +291,7 @@
 
 	for_each_cpu(i) {
 		void *scratch = *per_cpu_ptr(scratches, i);
-		if (scratch)
-			vfree(scratch);
+		vfree(scratch);
 	}
 
 	free_percpu(scratches);

[PATCH 4/4] resource release cleanup in net/ (take 2)

[Jesper Juhl]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2275 bytes --]

On Sun, 1 May 2005, Jesper Juhl wrote:

> On Sat, 30 Apr 2005, Arnaldo Carvalho de Melo wrote:
> 
> > On 4/30/05, Jesper Juhl <juhl-lkml@dif.dk> wrote:
> > > Hi David,
> > > 
> > > Since Andrew merged the patch that makes calling crypto_free_tfm() with a
> > > NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for
> > > NULL before calling that function, and while I was at it I removed similar
> > > redundant checks before calls to kfree() and vfree() in the same files.
> > > There are also a few tiny whitespace cleanups in there.
> > 
> > Jesper, I'd suggest that you left whitespaces for a separate patch, it
> > is always,
> > IMHO, better to have as small a patch as possible for reviewing.
> > 
> Sure thing. I've split the patches, and I believe that me going through 
> them a second time did them good, there are a few tiny changes over the 
> first version.
> 
> I split the patch in 4 parts (will send as replies to this mail) : 
> 	1) crypto_free_tfm related changes
> 	2) kfree related changes
> 	3) vfree related changes
> 	4) whitespace changes
> The whitespace changes ended up fairly bigger than initially. I expanded 
> the cleanup a bit. It's not a perfect, 100% complete cleanup, but it's IMO 
> a lot better than the originals.
> 

Here are the whitespace changes. Incremental patch on top of patch 3.
This bit got a little big (~80K), so instead of sending it inline I've 
bzip2 compressed it and attached it to the email.

Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
---

 drivers/net/wireless/airo.c                     | 1026 ++++++++++++------------
 drivers/net/wireless/hostap/hostap_crypt_ccmp.c |    4
 drivers/net/wireless/hostap/hostap_crypt_tkip.c |   25
 net/ieee80211/ieee80211_crypt_ccmp.c            |    3
 net/ieee80211/ieee80211_crypt_tkip.c            |   25
 net/ipv4/ah4.c                                  |    4
 net/ipv4/esp4.c                                 |    4
 net/ipv4/ipcomp.c                               |   11
 net/ipv6/addrconf.c                             |   20
 net/ipv6/ah6.c                                  |    1
 net/ipv6/esp6.c                                 |    4
 net/sctp/socket.c                               |    6
 12 files changed, 589 insertions(+), 544 deletions(-)


[-- Attachment #2: net-resource-freeing-cleanup-4-whitespace.patch.bz2 --]
[-- Type: APPLICATION/octet-stream, Size: 17101 bytes --]

Re: [PATCH 4/4] resource release cleanup in net/ (take 2)

[YOSHIFUJI Hideaki / 吉藤英明]

Hello.

In article <Pine.LNX.4.62.0505010352210.2094@dragon.hyggekrogen.localhost> (at Sun, 1 May 2005 03:55:37 +0200 (CEST)), Jesper Juhl <juhl-lkml@dif.dk> says:

> > 	4) whitespace changes
:

Please add me cc: next time...

>--- linux-2.6.12-rc3-mm1/net/ipv6/ah6.c.old3	2005-05-01 03:24:09.000000000 +0200
>+++ linux-2.6.12-rc3-mm1/net/ipv6/ah6.c	2005-05-01 03:24:20.000000000 +0200
>@@ -305,6 +305,7 @@
> 	skb_pull(skb, hdr_len);
> 	skb->h.raw = skb->data;
> 
>+
> 	kfree(tmp_hdr);
> 
> 	return nexthdr;

Why?

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
Homepage: http://www.yoshifuji.org/~hideaki/
GPG FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [PATCH 4/4] resource release cleanup in net/ (take 2)

[Jesper Juhl]

On Sun, 1 May 2005, YOSHIFUJI Hideaki / [iso-2022-jp] µÈÆ£±ÑÌÀ wrote:

> Hello.
> 
> In article <Pine.LNX.4.62.0505010352210.2094@dragon.hyggekrogen.localhost> (at Sun, 1 May 2005 03:55:37 +0200 (CEST)), Jesper Juhl <juhl-lkml@dif.dk> says:
> 
> > > 	4) whitespace changes
> :
> 
> Please add me cc: next time...
> 
> >--- linux-2.6.12-rc3-mm1/net/ipv6/ah6.c.old3	2005-05-01 03:24:09.000000000 +0200
> >+++ linux-2.6.12-rc3-mm1/net/ipv6/ah6.c	2005-05-01 03:24:20.000000000 +0200
> >@@ -305,6 +305,7 @@
> > 	skb_pull(skb, hdr_len);
> > 	skb->h.raw = skb->data;
> > 
> >+
> > 	kfree(tmp_hdr);
> > 
> > 	return nexthdr;
> 
> Why?
> 
Whoops, that was supposed to be "-" ...

-- 
Jesper

Re: [PATCH] resource release cleanup in net/

[Alexey Dobriyan]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Sat, Apr 30, 2005 at 10:36:00PM +0200, Jesper Juhl wrote:
> Since Andrew merged the patch that makes calling crypto_free_tfm() with a 
> NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for 
> NULL before calling that function

>  drivers/net/wireless/hostap/hostap_crypt_ccmp.c |    5 -
>  drivers/net/wireless/hostap/hostap_crypt_tkip.c |   10 +-
>  drivers/net/wireless/hostap/hostap_crypt_wep.c  |    5 -
>  net/ieee80211/ieee80211_crypt_ccmp.c            |    5 -
>  net/ieee80211/ieee80211_crypt_tkip.c            |   10 +-
>  net/ieee80211/ieee80211_crypt_wep.c             |    5 -

I think I have a better one for these.

[-- Attachment #2: hostap_free.patch --]
[-- Type: text/plain, Size: 6238 bytes --]

--- linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-05-01 01:53:50.000000000 +0000
+++ linux-2.6.12-rc3-mm1-hostap/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-05-01 02:21:10.000000000 +0000
@@ -102,17 +102,14 @@ static void * hostap_ccmp_init(int key_i
 	if (priv->tfm == NULL) {
 		printk(KERN_DEBUG "hostap_crypt_ccmp: could not allocate "
 		       "crypto API aes\n");
-		goto fail;
+		goto fail_free;
 	}
 
 	return priv;
 
+fail_free:
+	kfree(priv);
 fail:
-	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
-		kfree(priv);
-	}
 	module_put(THIS_MODULE);
 	return NULL;
 }
@@ -121,8 +118,7 @@ fail:
 static void hostap_ccmp_deinit(void *priv)
 {
 	struct hostap_ccmp_data *_priv = priv;
-	if (_priv && _priv->tfm)
-		crypto_free_tfm(_priv->tfm);
+	crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 	module_put(THIS_MODULE);
 }
--- linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_tkip.c	2005-05-01 01:53:50.000000000 +0000
+++ linux-2.6.12-rc3-mm1-hostap/drivers/net/wireless/hostap/hostap_crypt_tkip.c	2005-05-01 02:27:18.000000000 +0000
@@ -88,26 +88,23 @@ static void * hostap_tkip_init(int key_i
 	if (priv->tfm_arc4 == NULL) {
 		printk(KERN_DEBUG "hostap_crypt_tkip: could not allocate "
 		       "crypto API arc4\n");
-		goto fail;
+		goto fail_arc4;
 	}
 
 	priv->tfm_michael = crypto_alloc_tfm("michael_mic", 0);
 	if (priv->tfm_michael == NULL) {
 		printk(KERN_DEBUG "hostap_crypt_tkip: could not allocate "
 		       "crypto API michael_mic\n");
-		goto fail;
+		goto fail_michael;
 	}
 
 	return priv;
 
+fail_michael:
+	crypto_free_tfm(priv->tfm_arc4);
+fail_arc4:
+	kfree(priv);
 fail:
-	if (priv) {
-		if (priv->tfm_michael)
-			crypto_free_tfm(priv->tfm_michael);
-		if (priv->tfm_arc4)
-			crypto_free_tfm(priv->tfm_arc4);
-		kfree(priv);
-	}
 	module_put(THIS_MODULE);
 	return NULL;
 }
@@ -116,10 +113,8 @@ fail:
 static void hostap_tkip_deinit(void *priv)
 {
 	struct hostap_tkip_data *_priv = priv;
-	if (_priv && _priv->tfm_michael)
-		crypto_free_tfm(_priv->tfm_michael);
-	if (_priv && _priv->tfm_arc4)
-		crypto_free_tfm(_priv->tfm_arc4);
+	crypto_free_tfm(_priv->tfm_michael);
+	crypto_free_tfm(_priv->tfm_arc4);
 	kfree(priv);
 	module_put(THIS_MODULE);
 }
--- linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_wep.c	2005-05-01 01:53:50.000000000 +0000
+++ linux-2.6.12-rc3-mm1-hostap/drivers/net/wireless/hostap/hostap_crypt_wep.c	2005-05-01 02:30:08.000000000 +0000
@@ -59,7 +59,7 @@ static void * prism2_wep_init(int keyidx
 	if (priv->tfm == NULL) {
 		printk(KERN_DEBUG "hostap_crypt_wep: could not allocate "
 		       "crypto API arc4\n");
-		goto fail;
+		goto fail_tfm;
 	}
 
 	/* start WEP IV from a random value */
@@ -67,12 +67,9 @@ static void * prism2_wep_init(int keyidx
 
 	return priv;
 
+fail_tfm:
+	kfree(priv);
 fail:
-	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
-		kfree(priv);
-	}
 	module_put(THIS_MODULE);
 	return NULL;
 }
@@ -81,8 +78,7 @@ fail:
 static void prism2_wep_deinit(void *priv)
 {
 	struct prism2_wep_data *_priv = priv;
-	if (_priv && _priv->tfm)
-		crypto_free_tfm(_priv->tfm);
+	crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 	module_put(THIS_MODULE);
 }
--- linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_ccmp.c	2005-05-01 01:53:57.000000000 +0000
+++ linux-2.6.12-rc3-mm1-hostap/net/ieee80211/ieee80211_crypt_ccmp.c	2005-05-01 02:31:22.000000000 +0000
@@ -89,18 +89,14 @@ static void * ieee80211_ccmp_init(int ke
 	if (priv->tfm == NULL) {
 		printk(KERN_DEBUG "ieee80211_crypt_ccmp: could not allocate "
 		       "crypto API aes\n");
-		goto fail;
+		goto fail_tfm;
 	}
 
 	return priv;
 
+fail_tfm:
+	kfree(priv);
 fail:
-	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
-		kfree(priv);
-	}
-
 	return NULL;
 }
 
@@ -108,8 +104,7 @@ fail:
 static void ieee80211_ccmp_deinit(void *priv)
 {
 	struct ieee80211_ccmp_data *_priv = priv;
-	if (_priv && _priv->tfm)
-		crypto_free_tfm(_priv->tfm);
+	crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 }
 
--- linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_tkip.c	2005-05-01 01:53:57.000000000 +0000
+++ linux-2.6.12-rc3-mm1-hostap/net/ieee80211/ieee80211_crypt_tkip.c	2005-05-01 02:34:04.000000000 +0000
@@ -76,27 +76,23 @@ static void * ieee80211_tkip_init(int ke
 	if (priv->tfm_arc4 == NULL) {
 		printk(KERN_DEBUG "ieee80211_crypt_tkip: could not allocate "
 		       "crypto API arc4\n");
-		goto fail;
+		goto fail_arc4;
 	}
 
 	priv->tfm_michael = crypto_alloc_tfm("michael_mic", 0);
 	if (priv->tfm_michael == NULL) {
 		printk(KERN_DEBUG "ieee80211_crypt_tkip: could not allocate "
 		       "crypto API michael_mic\n");
-		goto fail;
+		goto fail_michael;
 	}
 
 	return priv;
 
+fail_michael:
+	crypto_free_tfm(priv->tfm_arc4);
+fail_arc4:
+	kfree(priv);
 fail:
-	if (priv) {
-		if (priv->tfm_michael)
-			crypto_free_tfm(priv->tfm_michael);
-		if (priv->tfm_arc4)
-			crypto_free_tfm(priv->tfm_arc4);
-		kfree(priv);
-	}
-
 	return NULL;
 }
 
@@ -104,10 +100,8 @@ fail:
 static void ieee80211_tkip_deinit(void *priv)
 {
 	struct ieee80211_tkip_data *_priv = priv;
-	if (_priv && _priv->tfm_michael)
-		crypto_free_tfm(_priv->tfm_michael);
-	if (_priv && _priv->tfm_arc4)
-		crypto_free_tfm(_priv->tfm_arc4);
+	crypto_free_tfm(_priv->tfm_michael);
+	crypto_free_tfm(_priv->tfm_arc4);
 	kfree(priv);
 }
 
--- linux-2.6.12-rc3-mm1/net/ieee80211/ieee80211_crypt_wep.c	2005-05-01 01:53:57.000000000 +0000
+++ linux-2.6.12-rc3-mm1-hostap/net/ieee80211/ieee80211_crypt_wep.c	2005-05-01 02:35:18.000000000 +0000
@@ -54,7 +54,7 @@ static void * prism2_wep_init(int keyidx
 	if (priv->tfm == NULL) {
 		printk(KERN_DEBUG "ieee80211_crypt_wep: could not allocate "
 		       "crypto API arc4\n");
-		goto fail;
+		goto fail_tfm;
 	}
 
 	/* start WEP IV from a random value */
@@ -62,12 +62,9 @@ static void * prism2_wep_init(int keyidx
 
 	return priv;
 
+fail_tfm:
+	kfree(priv);
 fail:
-	if (priv) {
-		if (priv->tfm)
-			crypto_free_tfm(priv->tfm);
-		kfree(priv);
-	}
 	return NULL;
 }
 
@@ -75,8 +72,7 @@ fail:
 static void prism2_wep_deinit(void *priv)
 {
 	struct prism2_wep_data *_priv = priv;
-	if (_priv && _priv->tfm)
-		crypto_free_tfm(_priv->tfm);
+	crypto_free_tfm(_priv->tfm);
 	kfree(priv);
 }
 

Re: [PATCH] resource release cleanup in net/

[Alexey Dobriyan]

On Sun, May 01, 2005 at 01:13:28AM +0200, Jesper Juhl wrote:
> On Sun, 1 May 2005, Alexey Dobriyan wrote:
> 
> > On Sat, Apr 30, 2005 at 10:36:00PM +0200, Jesper Juhl wrote:
> > > Since Andrew merged the patch that makes calling crypto_free_tfm() with a 
> > > NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for 
> > > NULL before calling that function
> > >  drivers/net/wireless/hostap/hostap_crypt_ccmp.c |    5 -
> > >  drivers/net/wireless/hostap/hostap_crypt_tkip.c |   10 +-
> > >  drivers/net/wireless/hostap/hostap_crypt_wep.c  |    5 -
> > >  net/ieee80211/ieee80211_crypt_ccmp.c            |    5 -
> > >  net/ieee80211/ieee80211_crypt_tkip.c            |   10 +-
> > >  net/ieee80211/ieee80211_crypt_wep.c             |    5 -
> > I think I have a better one for these.
> > 
> > --- linux-2.6.12-rc3-mm1/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-05-01 01:53:50.000000000 +0000
> > +++ linux-2.6.12-rc3-mm1-hostap/drivers/net/wireless/hostap/hostap_crypt_ccmp.c	2005-05-01 02:21:10.000000000 +0000

> > @@ -121,8 +118,7 @@ fail:
> >  static void hostap_ccmp_deinit(void *priv)
> >  {
> >  	struct hostap_ccmp_data *_priv = priv;
> > -	if (_priv && _priv->tfm)
> > -		crypto_free_tfm(_priv->tfm);
> > +	crypto_free_tfm(_priv->tfm);
> >  	kfree(priv);
> >  	module_put(THIS_MODULE);
> >  }
> 
> This will Oops if _priv is NULL. That's why my patch did 
> 
> if (_priv)
> 	crypto_free_tfm(_priv->tfm);

After hostap_ccmp_init() returns successfully:
1. priv is valid pointer	line 95
2. priv->tfm is valid pointer	line 102

Re: [PATCH] resource release cleanup in net/

[Herbert Xu]

On Sat, Apr 30, 2005 at 10:36:00PM +0200, Jesper Juhl wrote:
> 
> Since Andrew merged the patch that makes calling crypto_free_tfm() with a 
> NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for 
> NULL before calling that function, and while I was at it I removed similar 
> redundant checks before calls to kfree() and vfree() in the same files. 
> There are also a few tiny whitespace cleanups in there.

Hi Jesper:

Could we wait until the crypto_free_tfm patch gets into the main tree?
Most people here track that instead of mm.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] resource release cleanup in net/

[Jesper Juhl]

On Mon, 2 May 2005, Herbert Xu wrote:

> On Sat, Apr 30, 2005 at 10:36:00PM +0200, Jesper Juhl wrote:
> > 
> > Since Andrew merged the patch that makes calling crypto_free_tfm() with a 
> > NULL pointer safe into 2.6.12-rc3-mm1, I made a patch to remove checks for 
> > NULL before calling that function, and while I was at it I removed similar 
> > redundant checks before calls to kfree() and vfree() in the same files. 
> > There are also a few tiny whitespace cleanups in there.
> 
> Hi Jesper:
> 
> Could we wait until the crypto_free_tfm patch gets into the main tree?
> Most people here track that instead of mm.
> 

Sure. I don't have a problem with that.

I'll just hold on to the patches and resend them at a later date.


-- 
Jesper