* [PATCH] GFS2: Return -ENOMEM only if kmalloc fails
@ 2012-04-26 21:12 ` Emil Goode
0 siblings, 0 replies; 8+ messages in thread
From: Emil Goode @ 2012-04-26 21:12 UTC (permalink / raw)
To: swhiteho, cluster-devel; +Cc: linux-kernel, kernel-janitors, Emil Goode
The error variable should be assigned the value of -ENOMEM
after the NULL check and not before.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
fs/gfs2/acl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
index 230eb0f..90f6328 100644
--- a/fs/gfs2/acl.c
+++ b/fs/gfs2/acl.c
@@ -174,8 +174,8 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
len = posix_acl_to_xattr(acl, NULL, 0);
data = kmalloc(len, GFP_NOFS);
- error = -ENOMEM;
if (data == NULL)
+ error = -ENOMEM;
goto out;
posix_acl_to_xattr(acl, data, len);
error = gfs2_xattr_acl_chmod(ip, attr, data);
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] GFS2: Return -ENOMEM only if kmalloc fails
@ 2012-04-26 21:12 ` Emil Goode
0 siblings, 0 replies; 8+ messages in thread
From: Emil Goode @ 2012-04-26 21:12 UTC (permalink / raw)
To: swhiteho, cluster-devel; +Cc: linux-kernel, kernel-janitors, Emil Goode
The error variable should be assigned the value of -ENOMEM
after the NULL check and not before.
Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
fs/gfs2/acl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
index 230eb0f..90f6328 100644
--- a/fs/gfs2/acl.c
+++ b/fs/gfs2/acl.c
@@ -174,8 +174,8 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
len = posix_acl_to_xattr(acl, NULL, 0);
data = kmalloc(len, GFP_NOFS);
- error = -ENOMEM;
if (data = NULL)
+ error = -ENOMEM;
goto out;
posix_acl_to_xattr(acl, data, len);
error = gfs2_xattr_acl_chmod(ip, attr, data);
--
1.7.10
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] GFS2: Return -ENOMEM only if kmalloc fails
2012-04-26 21:12 ` Emil Goode
@ 2012-04-26 21:14 ` Oliver Neukum
-1 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2012-04-26 21:14 UTC (permalink / raw)
To: Emil Goode; +Cc: swhiteho, cluster-devel, linux-kernel, kernel-janitors
Am Donnerstag, 26. April 2012, 23:12:58 schrieb Emil Goode:
> The error variable should be assigned the value of -ENOMEM
> after the NULL check and not before.
>
> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> ---
> fs/gfs2/acl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index 230eb0f..90f6328 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
> @@ -174,8 +174,8 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
>
> len = posix_acl_to_xattr(acl, NULL, 0);
> data = kmalloc(len, GFP_NOFS);
> - error = -ENOMEM;
> if (data == NULL)
> + error = -ENOMEM;
> goto out;
Hint: read about the syntax of the if statement.
Secondly, consider how the compiler can optimize the original.
Regards
Oliver
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] GFS2: Return -ENOMEM only if kmalloc fails
@ 2012-04-26 21:14 ` Oliver Neukum
0 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2012-04-26 21:14 UTC (permalink / raw)
To: Emil Goode; +Cc: swhiteho, cluster-devel, linux-kernel, kernel-janitors
Am Donnerstag, 26. April 2012, 23:12:58 schrieb Emil Goode:
> The error variable should be assigned the value of -ENOMEM
> after the NULL check and not before.
>
> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> ---
> fs/gfs2/acl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index 230eb0f..90f6328 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
> @@ -174,8 +174,8 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
>
> len = posix_acl_to_xattr(acl, NULL, 0);
> data = kmalloc(len, GFP_NOFS);
> - error = -ENOMEM;
> if (data = NULL)
> + error = -ENOMEM;
> goto out;
Hint: read about the syntax of the if statement.
Secondly, consider how the compiler can optimize the original.
Regards
Oliver
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] GFS2: Return -ENOMEM only if kmalloc fails
2012-04-26 21:12 ` Emil Goode
@ 2012-04-26 21:19 ` Dave Jones
-1 siblings, 0 replies; 8+ messages in thread
From: Dave Jones @ 2012-04-26 21:19 UTC (permalink / raw)
To: Emil Goode; +Cc: swhiteho, cluster-devel, linux-kernel, kernel-janitors
On Thu, Apr 26, 2012 at 11:12:58PM +0200, Emil Goode wrote:
> The error variable should be assigned the value of -ENOMEM
> after the NULL check and not before.
>
> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> ---
> fs/gfs2/acl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index 230eb0f..90f6328 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
> @@ -174,8 +174,8 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
>
> len = posix_acl_to_xattr(acl, NULL, 0);
> data = kmalloc(len, GFP_NOFS);
> - error = -ENOMEM;
> if (data == NULL)
> + error = -ENOMEM;
> goto out;
> posix_acl_to_xattr(acl, data, len);
> error = gfs2_xattr_acl_chmod(ip, attr, data);
You missed the brackets on the if, introducing a bug that will cause it
to now always fail.
As 'error' gets overwritten in the successful case, there is no reason
to change this afaics.
Dave
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] GFS2: Return -ENOMEM only if kmalloc fails
@ 2012-04-26 21:19 ` Dave Jones
0 siblings, 0 replies; 8+ messages in thread
From: Dave Jones @ 2012-04-26 21:19 UTC (permalink / raw)
To: Emil Goode; +Cc: swhiteho, cluster-devel, linux-kernel, kernel-janitors
On Thu, Apr 26, 2012 at 11:12:58PM +0200, Emil Goode wrote:
> The error variable should be assigned the value of -ENOMEM
> after the NULL check and not before.
>
> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> ---
> fs/gfs2/acl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index 230eb0f..90f6328 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
> @@ -174,8 +174,8 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
>
> len = posix_acl_to_xattr(acl, NULL, 0);
> data = kmalloc(len, GFP_NOFS);
> - error = -ENOMEM;
> if (data = NULL)
> + error = -ENOMEM;
> goto out;
> posix_acl_to_xattr(acl, data, len);
> error = gfs2_xattr_acl_chmod(ip, attr, data);
You missed the brackets on the if, introducing a bug that will cause it
to now always fail.
As 'error' gets overwritten in the successful case, there is no reason
to change this afaics.
Dave
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] GFS2: Return -ENOMEM only if kmalloc fails
2012-04-26 21:14 ` Oliver Neukum
@ 2012-04-26 21:30 ` Emil Goode
-1 siblings, 0 replies; 8+ messages in thread
From: Emil Goode @ 2012-04-26 21:30 UTC (permalink / raw)
To: Oliver Neukum; +Cc: swhiteho, cluster-devel, linux-kernel, kernel-janitors
I forgot the braces, embarrassing!
I see your point, thanks.
Best regards, Emil
On Thu, 2012-04-26 at 23:14 +0200, Oliver Neukum wrote:
> Am Donnerstag, 26. April 2012, 23:12:58 schrieb Emil Goode:
> > The error variable should be assigned the value of -ENOMEM
> > after the NULL check and not before.
> >
> > Signed-off-by: Emil Goode <emilgoode@gmail.com>
> > ---
> > fs/gfs2/acl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> > index 230eb0f..90f6328 100644
> > --- a/fs/gfs2/acl.c
> > +++ b/fs/gfs2/acl.c
> > @@ -174,8 +174,8 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
> >
> > len = posix_acl_to_xattr(acl, NULL, 0);
> > data = kmalloc(len, GFP_NOFS);
> > - error = -ENOMEM;
> > if (data == NULL)
> > + error = -ENOMEM;
> > goto out;
>
> Hint: read about the syntax of the if statement.
> Secondly, consider how the compiler can optimize the original.
>
> Regards
> Oliver
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] GFS2: Return -ENOMEM only if kmalloc fails
@ 2012-04-26 21:30 ` Emil Goode
0 siblings, 0 replies; 8+ messages in thread
From: Emil Goode @ 2012-04-26 21:30 UTC (permalink / raw)
To: Oliver Neukum; +Cc: swhiteho, cluster-devel, linux-kernel, kernel-janitors
I forgot the braces, embarrassing!
I see your point, thanks.
Best regards, Emil
On Thu, 2012-04-26 at 23:14 +0200, Oliver Neukum wrote:
> Am Donnerstag, 26. April 2012, 23:12:58 schrieb Emil Goode:
> > The error variable should be assigned the value of -ENOMEM
> > after the NULL check and not before.
> >
> > Signed-off-by: Emil Goode <emilgoode@gmail.com>
> > ---
> > fs/gfs2/acl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> > index 230eb0f..90f6328 100644
> > --- a/fs/gfs2/acl.c
> > +++ b/fs/gfs2/acl.c
> > @@ -174,8 +174,8 @@ int gfs2_acl_chmod(struct gfs2_inode *ip, struct iattr *attr)
> >
> > len = posix_acl_to_xattr(acl, NULL, 0);
> > data = kmalloc(len, GFP_NOFS);
> > - error = -ENOMEM;
> > if (data = NULL)
> > + error = -ENOMEM;
> > goto out;
>
> Hint: read about the syntax of the if statement.
> Secondly, consider how the compiler can optimize the original.
>
> Regards
> Oliver
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-04-26 21:31 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-26 21:12 [PATCH] GFS2: Return -ENOMEM only if kmalloc fails Emil Goode
2012-04-26 21:12 ` Emil Goode
2012-04-26 21:14 ` Oliver Neukum
2012-04-26 21:14 ` Oliver Neukum
2012-04-26 21:30 ` Emil Goode
2012-04-26 21:30 ` Emil Goode
2012-04-26 21:19 ` Dave Jones
2012-04-26 21:19 ` Dave Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.