---
From: Borislav Petkov <[email protected]>
Fix error handling in ext3_create_journal according to kernel conventions.
Signed-off-by: Borislav Petkov <[email protected]>
--
Index: linux-2.6.22-rc6/fs/ext3/super.c
===================================================================
--- linux-2.6.22-rc6/fs/ext3/super.c.orig 2007-07-01 21:12:51.000000000 +0200
+++ linux-2.6.22-rc6/fs/ext3/super.c 2007-07-01 21:14:32.000000000 +0200
@@ -2075,6 +2075,7 @@
unsigned int journal_inum)
{
journal_t *journal;
+ int err;
if (sb->s_flags & MS_RDONLY) {
printk(KERN_ERR "EXT3-fs: readonly filesystem when trying to "
@@ -2082,13 +2083,15 @@
return -EROFS;
}
- if (!(journal = ext3_get_journal(sb, journal_inum)))
+ journal = ext3_get_journal(sb, journal_inum);
+ if (!journal)
return -EINVAL;
printk(KERN_INFO "EXT3-fs: creating new journal on inode %u\n",
journal_inum);
- if (journal_create(journal)) {
+ err = journal_create(journal);
+ if (err) {
printk(KERN_ERR "EXT3-fs: error creating journal.\n");
journal_destroy(journal);
return -EIO;
--
Regards/Gru?,
Boris.
On Mon, 2 Jul 2007 00:11:11 +0200
Borislav Petkov <[email protected]> wrote:
>
> ---
> From: Borislav Petkov <[email protected]>
>
> Fix error handling in ext3_create_journal according to kernel conventions.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> --
>
> Index: linux-2.6.22-rc6/fs/ext3/super.c
> ===================================================================
> --- linux-2.6.22-rc6/fs/ext3/super.c.orig 2007-07-01 21:12:51.000000000 +0200
> +++ linux-2.6.22-rc6/fs/ext3/super.c 2007-07-01 21:14:32.000000000 +0200
> @@ -2075,6 +2075,7 @@
> unsigned int journal_inum)
> {
> journal_t *journal;
> + int err;
>
> if (sb->s_flags & MS_RDONLY) {
> printk(KERN_ERR "EXT3-fs: readonly filesystem when trying to "
> @@ -2082,13 +2083,15 @@
> return -EROFS;
> }
>
> - if (!(journal = ext3_get_journal(sb, journal_inum)))
> + journal = ext3_get_journal(sb, journal_inum);
> + if (!journal)
> return -EINVAL;
>
> printk(KERN_INFO "EXT3-fs: creating new journal on inode %u\n",
> journal_inum);
>
> - if (journal_create(journal)) {
> + err = journal_create(journal);
> + if (err) {
> printk(KERN_ERR "EXT3-fs: error creating journal.\n");
> journal_destroy(journal);
> return -EIO;
Please prepare the equivalent patch for ext4. Without that, it'd probably
be better to avoid applying the ext3 patch: there are advantages to keeping
the two in sync where possible.
On Tue, Jul 03, 2007 at 03:25:43PM -0700, Andrew Morton wrote:
> Please prepare the equivalent patch for ext4. Without that, it'd probably
> be better to avoid applying the ext3 patch: there are advantages to keeping
> the two in sync where possible.
Hi Andrew,
here you go. By the way, this one is with generated diffstat. Do you prefer
it that way or is a diffstat for a patch touching a single file too much?
-----
From: Borislav Petkov <[email protected]>
Fix error handling in ext4_create_journal according to kernel conventions.
Signed-off-by: Borislav Petkov <[email protected]>
---
linux-2.6.22-rc7/fs/ext4/super.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Index: linux-2.6.22-rc7/fs/ext4/super.c
===================================================================
--- linux-2.6.22-rc7/fs/ext4/super.c.orig 2007-07-04 21:18:24.000000000 +0200
+++ linux-2.6.22-rc7/fs/ext4/super.c 2007-07-04 21:19:46.000000000 +0200
@@ -2150,6 +2150,7 @@
unsigned int journal_inum)
{
journal_t *journal;
+ int err;
if (sb->s_flags & MS_RDONLY) {
printk(KERN_ERR "EXT4-fs: readonly filesystem when trying to "
@@ -2157,13 +2158,15 @@
return -EROFS;
}
- if (!(journal = ext4_get_journal(sb, journal_inum)))
+ journal = ext4_get_journal(sb, journal_inum);
+ if (!journal)
return -EINVAL;
printk(KERN_INFO "EXT4-fs: creating new journal on inode %u\n",
journal_inum);
- if (jbd2_journal_create(journal)) {
+ err = jbd2_journal_create(journal);
+ if (err) {
printk(KERN_ERR "EXT4-fs: error creating journal.\n");
jbd2_journal_destroy(journal);
return -EIO;
--
Regards/Gru?,
Boris.
On Wed, Jul 04, 2007 at 09:30:24PM +0200, Borislav Petkov wrote:
> Fix error handling in ext4_create_journal according to kernel conventions.
> --- linux-2.6.22-rc7/fs/ext4/super.c.orig
> +++ linux-2.6.22-rc7/fs/ext4/super.c
> @@ -2150,6 +2150,7 @@
> unsigned int journal_inum)
> {
> journal_t *journal;
> + int err;
>
> if (sb->s_flags & MS_RDONLY) {
> printk(KERN_ERR "EXT4-fs: readonly filesystem when trying to "
> @@ -2157,13 +2158,15 @@
> return -EROFS;
> }
>
> - if (!(journal = ext4_get_journal(sb, journal_inum)))
> + journal = ext4_get_journal(sb, journal_inum);
> + if (!journal)
> return -EINVAL;
OK.
> printk(KERN_INFO "EXT4-fs: creating new journal on inode %u\n",
> journal_inum);
>
> - if (jbd2_journal_create(journal)) {
> + err = jbd2_journal_create(journal);
> + if (err) {
> printk(KERN_ERR "EXT4-fs: error creating journal.\n");
> jbd2_journal_destroy(journal);
> return -EIO;
Original code is fine.
On Thu, Jul 05, 2007 at 12:17:25AM +0400, Alexey Dobriyan wrote:
> On Wed, Jul 04, 2007 at 09:30:24PM +0200, Borislav Petkov wrote:
> > Fix error handling in ext4_create_journal according to kernel conventions.
>
> > --- linux-2.6.22-rc7/fs/ext4/super.c.orig
> > +++ linux-2.6.22-rc7/fs/ext4/super.c
> > @@ -2150,6 +2150,7 @@
> > unsigned int journal_inum)
> > {
> > journal_t *journal;
> > + int err;
> >
> > if (sb->s_flags & MS_RDONLY) {
> > printk(KERN_ERR "EXT4-fs: readonly filesystem when trying to "
> > @@ -2157,13 +2158,15 @@
> > return -EROFS;
> > }
> >
> > - if (!(journal = ext4_get_journal(sb, journal_inum)))
> > + journal = ext4_get_journal(sb, journal_inum);
> > + if (!journal)
> > return -EINVAL;
>
> OK.
>
> > printk(KERN_INFO "EXT4-fs: creating new journal on inode %u\n",
> > journal_inum);
> >
> > - if (jbd2_journal_create(journal)) {
> > + err = jbd2_journal_create(journal);
> > + if (err) {
> > printk(KERN_ERR "EXT4-fs: error creating journal.\n");
> > jbd2_journal_destroy(journal);
> > return -EIO;
>
> Original code is fine.
Hmm, ok but this is not the way error handling is done in the rest of ext3/4.
If you look for error variables declarations of type int in super.c, e.g. in
ext4_load_journal(), ext4_clear_journal_err() and ext4_remount() to name a few,
you'll see how the return value of the called function is assigned to the
error variable and then the last is checked in the if-conditional, which is the
usual way error handling is done in the kernel, imho. And my patch addresses exactly that.
--
Regards/Gru?,
Boris.