public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* sysfs backing store error path confusion
@ 2004-11-02  8:46 Milton Miller
  2004-11-02 16:03 ` Maneesh Soni
  0 siblings, 1 reply; 6+ messages in thread
From: Milton Miller @ 2004-11-02  8:46 UTC (permalink / raw)
  To: Andrew Morton, Greg Kroah-Hartman, Maneesh Soni; +Cc: linux-kernel

sysfs_new_dirent returns ERR_PTR(-ENOMEM) if kmalloc fails but the callers
were expecting NULL.  

Compile & link tested.  (Yes, changing the callee would be a smaller change).

===== fs/sysfs/dir.c 1.24 vs edited =====
--- 1.24/fs/sysfs/dir.c	2004-11-01 21:46:46 +01:00
+++ edited/fs/sysfs/dir.c	2004-11-02 09:12:31 +01:00
@@ -55,8 +55,8 @@ int sysfs_make_dirent(struct sysfs_diren
 	struct sysfs_dirent * sd;
 
 	sd = sysfs_new_dirent(parent_sd, element);
-	if (!sd)
-		return -ENOMEM;
+	if (IS_ERR(sd))
+		return PTR_ERR(sd);
 
 	sd->s_mode = mode;
 	sd->s_type = type;
@@ -332,13 +332,18 @@ static int sysfs_dir_open(struct inode *
 {
 	struct dentry * dentry = file->f_dentry;
 	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
+	struct sysfs_dirent * sd;
 
 	down(&dentry->d_inode->i_sem);
-	file->private_data = sysfs_new_dirent(parent_sd, NULL);
+	sd = sysfs_new_dirent(parent_sd, NULL);
 	up(&dentry->d_inode->i_sem);
 
-	return file->private_data ? 0 : -ENOMEM;
+	if (IS_ERR(sd))
+		return PTR_ERR(sd);
+	
+	file->private_data = sd;
 
+	return 0;
 }
 
 static int sysfs_dir_close(struct inode *inode, struct file *file)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sysfs backing store error path confusion
  2004-11-02  8:46 Milton Miller
@ 2004-11-02 16:03 ` Maneesh Soni
  2004-11-03 21:42   ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Maneesh Soni @ 2004-11-02 16:03 UTC (permalink / raw)
  To: Milton Miller; +Cc: Andrew Morton, Greg Kroah-Hartman, linux-kernel

On Tue, Nov 02, 2004 at 02:46:58AM -0600, Milton Miller wrote:
> sysfs_new_dirent returns ERR_PTR(-ENOMEM) if kmalloc fails but the callers
> were expecting NULL.  
> 
> Compile & link tested.  (Yes, changing the callee would be a smaller change).
> 

Thanks for spotting this. But as you said, I will prefer to change the callee.
How about this patch? 

Andrew, Greg, please include this if found ok.

Thanks
Maneesh



o sysfs_new_dirent to retrun NULL if kmalloc fails. Thanks to Milton Miller 
  for spotting this.

Signed-off-by: <maneesh@in.ibm.com>
---

 linux-2.6.10-rc1-bk12-maneesh/fs/sysfs/dir.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN fs/sysfs/dir.c~fix-sysfs_new_dirent-return fs/sysfs/dir.c
--- linux-2.6.10-rc1-bk12/fs/sysfs/dir.c~fix-sysfs_new_dirent-return	2004-11-02 08:38:57.000000000 -0600
+++ linux-2.6.10-rc1-bk12-maneesh/fs/sysfs/dir.c	2004-11-02 09:17:18.000000000 -0600
@@ -38,7 +38,7 @@ static struct sysfs_dirent * sysfs_new_d
 
 	sd = kmalloc(sizeof(*sd), GFP_KERNEL);
 	if (!sd)
-		return -ENOMEM;
+		return NULL;
 
 	memset(sd, 0, sizeof(*sd));
 	atomic_set(&sd->s_count, 1);
_

-- 
Maneesh Soni
Linux Technology Center, 
IBM Austin
email: maneesh@in.ibm.com
Phone: 1-512-838-1896 Fax: 
T/L : 6781896

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sysfs backing store error path confusion
  2004-11-02 16:03 ` Maneesh Soni
@ 2004-11-03 21:42   ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2004-11-03 21:42 UTC (permalink / raw)
  To: Maneesh Soni; +Cc: Milton Miller, Andrew Morton, linux-kernel

On Tue, Nov 02, 2004 at 10:03:34AM -0600, Maneesh Soni wrote:
> On Tue, Nov 02, 2004 at 02:46:58AM -0600, Milton Miller wrote:
> > sysfs_new_dirent returns ERR_PTR(-ENOMEM) if kmalloc fails but the callers
> > were expecting NULL.  
> > 
> > Compile & link tested.  (Yes, changing the callee would be a smaller change).
> > 
> 
> Thanks for spotting this. But as you said, I will prefer to change the callee.
> How about this patch? 
> 
> Andrew, Greg, please include this if found ok.
> 
> Thanks
> Maneesh
> 
> 
> 
> o sysfs_new_dirent to retrun NULL if kmalloc fails. Thanks to Milton Miller 
>   for spotting this.
> 
> Signed-off-by: <maneesh@in.ibm.com>
> ---
> 
>  linux-2.6.10-rc1-bk12-maneesh/fs/sysfs/dir.c |    2 +-
>  1 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff -puN fs/sysfs/dir.c~fix-sysfs_new_dirent-return fs/sysfs/dir.c
> --- linux-2.6.10-rc1-bk12/fs/sysfs/dir.c~fix-sysfs_new_dirent-return	2004-11-02 08:38:57.000000000 -0600
> +++ linux-2.6.10-rc1-bk12-maneesh/fs/sysfs/dir.c	2004-11-02 09:17:18.000000000 -0600
> @@ -38,7 +38,7 @@ static struct sysfs_dirent * sysfs_new_d
>  
>  	sd = kmalloc(sizeof(*sd), GFP_KERNEL);
>  	if (!sd)
> -		return -ENOMEM;
> +		return NULL;

Actually, this needs to be a 0, not NULL, otherwise the compiler
complains with a warning.  I've fixed it up and applied it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sysfs backing store error path confusion
@ 2004-11-05  7:49 Milton Miller
  2004-11-06  6:20 ` Maneesh Soni
  2004-11-12 20:49 ` Greg KH
  0 siblings, 2 replies; 6+ messages in thread
From: Milton Miller @ 2004-11-05  7:49 UTC (permalink / raw)
  To: maneesh, greg; +Cc: akpm, linux-kernel


On Nov 3, 2004, at 3:42 PM, Greg KH wrote:

|On Tue, Nov 02, 2004 at 10:03:34AM -0600, Maneesh Soni wrote:
||On Tue, Nov 02, 2004 at 02:46:58AM -0600, Milton Miller wrote:
|||sysfs_new_dirent returns ERR_PTR(-ENOMEM) if kmalloc fails but the callers
|||were expecting NULL.  
||
||Thanks for spotting this. But as you said, I will prefer to change the callee.
||How about this patch? 
..
||-		return -ENOMEM;
||+		return NULL;
|
|Actually, this needs to be a 0, not NULL, otherwise the compiler
|complains with a warning.  I've fixed it up and applied it.
|
|thanks,
|
|greg k-h

I wondered why greg thought the type was wrong.   After it was merged I 
realized that the wrong function was changed.  Here's an attempt to fix
both errors.

milton

===== fs/sysfs/dir.c 1.27 vs edited =====
--- 1.27/fs/sysfs/dir.c	2004-11-04 22:37:32 +01:00
+++ edited/fs/sysfs/dir.c	2004-11-05 08:10:54 +01:00
@@ -38,7 +38,7 @@ static struct sysfs_dirent * sysfs_new_d
 
 	sd = kmalloc(sizeof(*sd), GFP_KERNEL);
 	if (!sd)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	memset(sd, 0, sizeof(*sd));
 	atomic_set(&sd->s_count, 1);
@@ -56,7 +56,7 @@ int sysfs_make_dirent(struct sysfs_diren
 
 	sd = sysfs_new_dirent(parent_sd, element);
 	if (!sd)
-		return 0;
+		return -ENOMEM;
 
 	sd->s_mode = mode;
 	sd->s_type = type;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sysfs backing store error path confusion
  2004-11-05  7:49 sysfs backing store error path confusion Milton Miller
@ 2004-11-06  6:20 ` Maneesh Soni
  2004-11-12 20:49 ` Greg KH
  1 sibling, 0 replies; 6+ messages in thread
From: Maneesh Soni @ 2004-11-06  6:20 UTC (permalink / raw)
  To: Milton Miller; +Cc: Greg KH, Andrew Morton, linux-kernel

On Fri, Nov 05, 2004 at 01:49:34AM -0600, Milton Miller wrote:
> 
> On Nov 3, 2004, at 3:42 PM, Greg KH wrote:
> 
> |On Tue, Nov 02, 2004 at 10:03:34AM -0600, Maneesh Soni wrote:
> ||On Tue, Nov 02, 2004 at 02:46:58AM -0600, Milton Miller wrote:
> |||sysfs_new_dirent returns ERR_PTR(-ENOMEM) if kmalloc fails but the callers
> |||were expecting NULL.  
> ||
> ||Thanks for spotting this. But as you said, I will prefer to change the callee.
> ||How about this patch? 
> ..
> ||-		return -ENOMEM;
> ||+		return NULL;
> |
> |Actually, this needs to be a 0, not NULL, otherwise the compiler
> |complains with a warning.  I've fixed it up and applied it.
> |
> |thanks,
> |
> |greg k-h
> 
> I wondered why greg thought the type was wrong.   After it was merged I 
> realized that the wrong function was changed.  Here's an attempt to fix
> both errors.
> 

Yes, it is correct now. Sorry about the confusion. I edited at the wrong
palce.

Thanks
Maneesh

> 
> ===== fs/sysfs/dir.c 1.27 vs edited =====
> --- 1.27/fs/sysfs/dir.c	2004-11-04 22:37:32 +01:00
> +++ edited/fs/sysfs/dir.c	2004-11-05 08:10:54 +01:00
> @@ -38,7 +38,7 @@ static struct sysfs_dirent * sysfs_new_d
>  
>  	sd = kmalloc(sizeof(*sd), GFP_KERNEL);
>  	if (!sd)
> -		return ERR_PTR(-ENOMEM);
> +		return NULL;
>  
>  	memset(sd, 0, sizeof(*sd));
>  	atomic_set(&sd->s_count, 1);
> @@ -56,7 +56,7 @@ int sysfs_make_dirent(struct sysfs_diren
>  
>  	sd = sysfs_new_dirent(parent_sd, element);
>  	if (!sd)
> -		return 0;
> +		return -ENOMEM;
>  
>  	sd->s_mode = mode;
>  	sd->s_type = type;
> 

-- 
Maneesh Soni
Linux Technology Center, 
IBM Austin
email: maneesh@in.ibm.com
Phone: 1-512-838-1896 Fax: 
T/L : 6781896

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sysfs backing store error path confusion
  2004-11-05  7:49 sysfs backing store error path confusion Milton Miller
  2004-11-06  6:20 ` Maneesh Soni
@ 2004-11-12 20:49 ` Greg KH
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2004-11-12 20:49 UTC (permalink / raw)
  To: Milton Miller; +Cc: Maneesh Soni, Andrew Morton, linux-kernel

On Fri, Nov 05, 2004 at 01:49:34AM -0600, Milton Miller wrote:
> 
> On Nov 3, 2004, at 3:42 PM, Greg KH wrote:
> 
> |On Tue, Nov 02, 2004 at 10:03:34AM -0600, Maneesh Soni wrote:
> ||On Tue, Nov 02, 2004 at 02:46:58AM -0600, Milton Miller wrote:
> |||sysfs_new_dirent returns ERR_PTR(-ENOMEM) if kmalloc fails but the callers
> |||were expecting NULL.  
> ||
> ||Thanks for spotting this. But as you said, I will prefer to change the callee.
> ||How about this patch? 
> ..
> ||-		return -ENOMEM;
> ||+		return NULL;
> |
> |Actually, this needs to be a 0, not NULL, otherwise the compiler
> |complains with a warning.  I've fixed it up and applied it.
> |
> |thanks,
> |
> |greg k-h
> 
> I wondered why greg thought the type was wrong.   After it was merged I 
> realized that the wrong function was changed.  Here's an attempt to fix
> both errors.
> 
> milton

Applied, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2004-11-12 21:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-11-05  7:49 sysfs backing store error path confusion Milton Miller
2004-11-06  6:20 ` Maneesh Soni
2004-11-12 20:49 ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2004-11-02  8:46 Milton Miller
2004-11-02 16:03 ` Maneesh Soni
2004-11-03 21:42   ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox