lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <alpine.LFD.0.999.0711061334030.15101@woody.linux-foundation.org>
Date:	Tue, 6 Nov 2007 13:38:09 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Dirk Hohndel <hohndel@...ux.intel.com>
cc:	Bob Copeland <me@...copeland.com>,
	Jens Axboe <jens.axboe@...cle.com>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	Andries Brouwer <aeb@....nl>, Al Viro <viro@....linux.org.uk>,
	linux-kernel@...r.kernel.org
Subject: Re: [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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ