[PATCH] Staging: lustre: llite: Remove NULL check before kfree

[PATCH] Staging: lustre: llite: Remove NULL check before kfree

[Bhumika Goyal]

NULL check before kfree is unnecessary so remove it.
Semantic patch used:
// <smpl>
@@ expression E; @@
- if (E != NULL) { kfree(E); }
+ kfree(E);
@@ expression E; @@
- if (E != NULL) { kfree(E); E = NULL; }
+ kfree(E);
+ E = NULL;
// </smpl>

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/staging/lustre/lustre/llite/xattr.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
index 8eb43f1..310e381 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -192,11 +192,7 @@ int ll_setxattr_common(struct inode *inode, const char *name,
 			 valid, name, pv, size, 0, flags,
 			 ll_i2suppgid(inode), &req);
 #ifdef CONFIG_FS_POSIX_ACL
-	if (new_value != NULL)
-		/*
-		 * Release the posix ACL space.
-		 */
-		kfree(new_value);
+	kfree(new_value);
 	if (acl != NULL)
 		lustre_ext_acl_xattr_free(acl);
 #endif
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: lustre: llite: Remove NULL check before kfree

[Julia Lawall]

On Fri, 12 Feb 2016, Bhumika Goyal wrote:

> NULL check before kfree is unnecessary so remove it.
> Semantic patch used:
> // <smpl>
> @@ expression E; @@
> - if (E != NULL) { kfree(E); }

Normally, the braces in this rule should not be useful, because the kernel 
coding style is that a single statement in an if branch should not be 
enclosed in braces, at least unless another branch is present and that one 
requires braces.  But here there is no other branch.

However, as you see in your example, when you put a single statement with 
braces, Coccinelle will also consider the case where the braces are not 
there, via an isomoprhism.

> + kfree(E);
> @@ expression E; @@
> - if (E != NULL) { kfree(E); E = NULL; }
> + kfree(E);
> + E = NULL;
> // </smpl>
> 
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---
>  drivers/staging/lustre/lustre/llite/xattr.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
> index 8eb43f1..310e381 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -192,11 +192,7 @@ int ll_setxattr_common(struct inode *inode, const char *name,
>  			 valid, name, pv, size, 0, flags,
>  			 ll_i2suppgid(inode), &req);
>  #ifdef CONFIG_FS_POSIX_ACL
> -	if (new_value != NULL)
> -		/*
> -		 * Release the posix ACL space.
> -		 */
> -		kfree(new_value);
> +	kfree(new_value);

I'm not sure that it's a good idea to remove the comment.  Coccinelle has 
a strategy for removing comments, but it is just a heuristic.  There is no 
guarantee that it makes the right decision in all cases.

Concretely, the comment is removed because you remoce both the if header 
and the kfree.  Coccinelle doesn't realize that you added the kfree back.

julia

>  	if (acl != NULL)
>  		lustre_ext_acl_xattr_free(acl);
>  #endif
> -- 
> 1.9.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1455272748-14400-1-git-send-email-bhumirks%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH] Staging: lustre: llite: Remove NULL check before kfree

[Bhumika Goyal]

[-- Attachment #1.1: Type: text/plain, Size: 64 bytes --]

I will send a v2 with the comments added back.

Thanks,
Bhumika

[-- Attachment #1.2: Type: text/html, Size: 97 bytes --]

[PATCH v2] Staging: lustre: llite: Remove NULL check before kfree

[Bhumika Goyal]

NULL check before kfree is unnecessary so remove it.
Semantic patch used:
// <smpl>
@@ expression E; @@
- if (E != NULL) { kfree(E); }
+ kfree(E);
@@ expression E; @@
- if (E != NULL) { kfree(E); E = NULL; }
+ kfree(E);
+ E = NULL;
// </smpl>

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
Changes since v1:
*Add the comment before kfree back.
---
 drivers/staging/lustre/lustre/llite/xattr.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
index 8eb43f1..fa477e7 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -192,11 +192,10 @@ int ll_setxattr_common(struct inode *inode, const char *name,
 			 valid, name, pv, size, 0, flags,
 			 ll_i2suppgid(inode), &req);
 #ifdef CONFIG_FS_POSIX_ACL
-	if (new_value != NULL)
-		/*
-		 * Release the posix ACL space.
-		 */
-		kfree(new_value);
+	/*
+	 * Release the posix ACL space.
+	 */
+	kfree(new_value);
 	if (acl != NULL)
 		lustre_ext_acl_xattr_free(acl);
 #endif
-- 
1.9.1

Re: [Outreachy kernel] [PATCH v2] Staging: lustre: llite: Remove NULL check before kfree

[Julia Lawall]

On Fri, 12 Feb 2016, Bhumika Goyal wrote:

> NULL check before kfree is unnecessary so remove it.
> Semantic patch used:
> // <smpl>
> @@ expression E; @@
> - if (E != NULL) { kfree(E); }
> + kfree(E);
> @@ expression E; @@
> - if (E != NULL) { kfree(E); E = NULL; }
> + kfree(E);
> + E = NULL;
> // </smpl>
>
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---
> Changes since v1:
> *Add the comment before kfree back.
> ---

The --- below your comemnt is not necessary.

julia

>  drivers/staging/lustre/lustre/llite/xattr.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
> index 8eb43f1..fa477e7 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -192,11 +192,10 @@ int ll_setxattr_common(struct inode *inode, const char *name,
>  			 valid, name, pv, size, 0, flags,
>  			 ll_i2suppgid(inode), &req);
>  #ifdef CONFIG_FS_POSIX_ACL
> -	if (new_value != NULL)
> -		/*
> -		 * Release the posix ACL space.
> -		 */
> -		kfree(new_value);
> +	/*
> +	 * Release the posix ACL space.
> +	 */
> +	kfree(new_value);
>  	if (acl != NULL)
>  		lustre_ext_acl_xattr_free(acl);
>  #endif
> --
> 1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1455287874-18123-1-git-send-email-bhumirks%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH v2] Staging: lustre: llite: Remove NULL check before kfree

[Bhumika Goyal]

[-- Attachment #1.1: Type: text/plain, Size: 73 bytes --]

Thank you. Will surely keep in mind for future patches.

Thanks,
Bhumika

[-- Attachment #1.2: Type: text/html, Size: 106 bytes --]

Re: [Outreachy kernel] [PATCH v2] Staging: lustre: llite: Remove NULL check before kfree

[Julia Lawall]

On Fri, 12 Feb 2016, Bhumika Goyal wrote:

> Thank you. Will surely keep in mind for future patches.

It is nice to quote some of the message that you are responding to.  
Otherwise, it is hard to follow the conversation.

julia

> 
> Thanks,
> Bhumika
> 
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/710bbc9f-1914-4b29-b135-
> a229028e47ca%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
> 
>