On 02/09/2015 06:22 PM, Nicholas Krause wrote:
Adds error handling to the function add_disk. Error checking is
added by removing the calls to WARN_ON when there is a failure in
various places in the function to proper return statements with
a error code. Further more due to this we must also add a statement
to return 0 at the end of the function to signal success to the
caller of the function to notify it that the function,add_disk has
succeeded and change the function's return type to a int now to
allow returning of error codes for the function,add_disk to be
allowed.
There's a lot wrong with this still:
@@ -595,10 +594,9 @@ void add_disk(struct gendisk *disk)
disk->flags |= GENHD_FL_UP;
retval = blk_alloc_devt(&disk->part0, &devt);
- if (retval) {
- WARN_ON(1);
- return;
- }
+ if (retval)
+ return -ENOMEM;
Why isn't that returning retval?
+
disk_to_dev(disk)->devt = devt;
/* ->major and ->first_minor aren't supposed to be
@@ -622,13 +620,16 @@ void add_disk(struct gendisk *disk)
* Take an extra ref on queue which will be put on disk_release()
* so that it sticks around as long as @disk is there.
*/
- WARN_ON_ONCE(!blk_get_queue(disk->queue));
+ if (!blk_get_queue(disk->queue))
+ return -ENODEV;
You just leaked the devt alloc.
retval = sysfs_create_link(&disk_to_dev(disk)->kobj, &bdi->dev->kobj,
"bdi");
- WARN_ON(retval);
-
+ if (retval)
+ return -EFAULT;
Leaked devt alloc and queue ref.
On top of that, some of the functions that add_disk() calls can fail,
yet it doesn't check for those. And the most major part of this is
ensuring that all callders of add_disk() properly check and handle the
error it will pass back, that is completely ignored. Just making
add_disk() error and unroll itself is the smaller part of the task.
--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
