On Sat, 28 Apr 2007 13:30:23 +0800 WANG Cong <[EMAIL PROTECTED]> wrote:

> Since 'sysfs_create_file' is declared with attribute warn_unused_result, we 
> must always check its return value carefully.
> 

Well that's not really the reason for your patch.

warn_unused_result is there to tell us that there are deeper problems in
the code which need addressing: the failure to check the
sysfs_create_file() return value means that bugs in the kernel can remain
undetected, or can be harder to find.

> 
> ---
> 
> --- linux-2.6.21-rc7-mm2/drivers/block/nbd.c.orig     2007-04-27 
> 17:27:47.000000000 +0800
> +++ linux-2.6.21-rc7-mm2/drivers/block/nbd.c  2007-04-27 17:47:32.000000000 
> +0800
> @@ -373,7 +373,10 @@ static void nbd_do_it(struct nbd_device 
>       BUG_ON(lo->magic != LO_MAGIC);
>  
>       lo->pid = current->pid;
> -     sysfs_create_file(&lo->disk->kobj, &pid_attr.attr);
> +     if (sysfs_create_file(&lo->disk->kobj, &pid_attr.attr)) {
> +             printk(KERN_ERR "nbd: sysfs_create_file failed!");
> +             return;
> +     }
>  
>       while ((req = nbd_read_stat(lo)) != NULL)
>               nbd_end_request(req);

It would better saner to propagate this error back through callers:

--- a/drivers/block/nbd.c~nbd-check-the-return-value-of-sysfs_create_file-fix
+++ a/drivers/block/nbd.c
@@ -366,23 +366,25 @@ static struct disk_attribute pid_attr = 
        .show = pid_show,
 };
 
-static void nbd_do_it(struct nbd_device *lo)
+static int nbd_do_it(struct nbd_device *lo)
 {
        struct request *req;
+       int ret;
 
        BUG_ON(lo->magic != LO_MAGIC);
 
        lo->pid = current->pid;
-       if (sysfs_create_file(&lo->disk->kobj, &pid_attr.attr)) {
+       ret = sysfs_create_file(&lo->disk->kobj, &pid_attr.attr);
+       if (ret) {
                printk(KERN_ERR "nbd: sysfs_create_file failed!");
-               return;
+               return ret;
        }
 
        while ((req = nbd_read_stat(lo)) != NULL)
                nbd_end_request(req);
 
        sysfs_remove_file(&lo->disk->kobj, &pid_attr.attr);
-       return;
+       return 0;
 }
 
 static void nbd_clear_que(struct nbd_device *lo)
@@ -572,7 +574,9 @@ static int nbd_ioctl(struct inode *inode
        case NBD_DO_IT:
                if (!lo->file)
                        return -EINVAL;
-               nbd_do_it(lo);
+               error = nbd_do_it(lo);
+               if (error)
+                       return error;
                /* on return tidy up in case we have a signal */
                /* Forcibly shutdown the socket causing all listeners
                 * to error
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to