Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755426AbXKFVkp (ORCPT ); Tue, 6 Nov 2007 16:40:45 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755267AbXKFVkP (ORCPT ); Tue, 6 Nov 2007 16:40:15 -0500 Received: from smtp2.linux-foundation.org ([207.189.120.14]:34947 "EHLO smtp2.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755116AbXKFVkM (ORCPT ); Tue, 6 Nov 2007 16:40:12 -0500 Date: Tue, 6 Nov 2007 13:38:09 -0800 (PST) From: Linus Torvalds To: Dirk Hohndel cc: Bob Copeland , Jens Axboe , Cornelia Huck , Andries Brouwer , Al Viro , linux-kernel@vger.kernel.org Subject: Re: [PATCH] FS: add_partition silently ignored errors In-Reply-To: <20071106200259.GA3879@linux.intel.com> Message-ID: References: <20071029154849.GA24187@bigserver.hohndel.org> <20071030080742.GE4993@kernel.dk> <20071030100934.6d2a8f12@gondolin.boeblingen.de.ibm.com> <20071030165608.GA2601@linux.intel.com> <20071030183112.7e860c23@gondolin.boeblingen.de.ibm.com> <20071030225635.GA3401@linux.intel.com> <20071102130438.GC28340@kernel.dk> <20071102192914.GA888@linux.intel.com> <20071102202949.GA1128@linux.intel.com> <20071106200259.GA3879@linux.intel.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1703 Lines: 53 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/