Re: [PATCH] block:Add proper error handling to the function, disk_add_events

[Jens Axboe]

On 05/14/2015 09:39 PM, Nicholas Krause wrote:
> On May 14, 2015 9:22:22 PM EDT, Jens Axboe <axboe@xxxxxxxxx> wrote:
> > On 05/14/2015 07:57 PM, Nicholas Krause wrote:
> > > This adds the proper required error checking to the function,
> > > disk_add_events for  when there are no disk events by returning
> > > the error code, -EBUSY. Further this also adds error checking
> > > for when our call to the function, sysfs_create_files by making
> > > this function's return now go into a newly declared variable,
> > > ret and at the end of this function's body return it to indicate
> > > whether this function is successful or not to the caller.
> > >
> > > Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx>
> > > ---
> > >    block/genhd.c | 13 +++++++++----
> > >    1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/block/genhd.c b/block/genhd.c
> > > index 0a536dc..3eb7ee9 100644
> > > --- a/block/genhd.c
> > > +++ b/block/genhd.c
> > > @@ -1803,15 +1803,19 @@ static void disk_alloc_events(struct gendisk
> > *disk)
> > >    	disk->ev = ev;
> > >    }
> > >
> > > -static void disk_add_events(struct gendisk *disk)
> > > +static int disk_add_events(struct gendisk *disk)
> > >    {
> > > +	int ret = 0;
> > >    	if (!disk->ev)
> > > -		return;
> > > +		return -EBUSY;
> > > +
> > > +	ret = sysfs_create_files(&disk_to_dev(disk)->kobj,
> > disk_events_attrs)
> > > -	/* FIXME: error handling */
> > > -	if (sysfs_create_files(&disk_to_dev(disk)->kobj, disk_events_attrs)
> > < 0)
> > > +	if (!ret) {
> > >    		pr_warn("%s: failed to create sysfs files for events\n",
> > >    			disk->disk_name);
> > > +		return ret;
> > > +	}
> >
> > You didn't even test this, obviously.
> It builds on my system.  I can't see anything wrong with it, please explain.

The fact that it compiles does not constitute that it has been tested. 
And it's definitely broken, as a test boot would have revealed.

--
Jens Axboe

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