lkml.org 
[lkml]   [2007]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] FS: add_partition silently ignored errors


On Tue, 6 Nov 2007, Dirk Hohndel wrote:
>
> (since there was no more discussion, here's the last version that includes
> the requested change to not fail if the partition is larger than the disk)

I'd suggest keeping it in -mm for a while.

I worry about these kinds of "trivial" changes. Quite often, some errors
may be normal, and breaking out early can sometimes hurt more than it
helps, in that it just makes things not even limp along. That said, with
the disk/partition size check removed, this looks more palatable.

> - add_partition(disk, part, start, length, ADDPART_FLAG_NONE);
> + err = add_partition(disk, part, start, length,
> + ADDPART_FLAG_NONE)
> + if (err) {
> + mutex_unlock(&bdev->bd_mutex);
> + return err;
> + }
> mutex_unlock(&bdev->bd_mutex);
> return 0;

However, this is just ugly. The thing is not only apparently totally
untested, since it is missing a semicolon, it should also just read

err = add_partition(disk, part, start, length, ADDPART_FLAG_NONE);
mutex_unlock(&bdev->bd_mutex);
return err;

without any unnecessary conditionals (or ugly line-breaks, for that
matter).

> - sysfs_create_file(&p->kobj, &addpartattr);
> + err = sysfs_create_file(&p->kobj, &addpartattr);
> + if (err) {
> + sysfs_remove_link(&p->kobj, "subsystem");
> + goto out_cleanup;

Wouldn't this be better done as

if (err)
goto out_unlink_cleanup;

to match the rest of the error handling?

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

\
 
 \ /
  Last update: 2007-11-06 22:43    [W:0.125 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site