[PATCH] ext2: Make sb magic number endianness conversion consistent

[PATCH] ext2: Make sb magic number endianness conversion consistent

[Carlos Maiolino]

All ext2_super_block fields are converted from le16 to cpu when checked or
loaded into memory. Magic number check is the only field where we convert the
magic number to le16. So, make the conversion pattern consistent with the
remaining code

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/ext2/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 2885349..808f99f 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -927,7 +927,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 		}
 		es = (struct ext2_super_block *) (((char *)bh->b_data) + offset);
 		sbi->s_es = es;
-		if (es->s_magic != cpu_to_le16(EXT2_SUPER_MAGIC)) {
+		if (le16_to_cpu(es->s_magic) != EXT2_SUPER_MAGIC) {
 			ext2_msg(sb, KERN_ERR, "error: magic mismatch");
 			goto failed_mount;
 		}
-- 
1.8.1.4

Re: [PATCH] ext2: Make sb magic number endianness conversion consistent

[Lukáš Czerner]

On Thu, 17 Oct 2013, Carlos Maiolino wrote:

> Date: Thu, 17 Oct 2013 14:24:58 -0300
> From: Carlos Maiolino <cmaiolino@redhat.com>
> To: linux-ext4@vger.kernel.org
> Subject: [PATCH] ext2: Make sb magic number endianness conversion consistent
> 
> All ext2_super_block fields are converted from le16 to cpu when checked or
> loaded into memory. Magic number check is the only field where we convert the
> magic number to le16. So, make the conversion pattern consistent with the
> remaining code

This not exactly true since this kind of comparison when a constant
is converted rather the variable is present on several places in
ext2, ext3 and ext4 (ext3 and ext4 have the same check you're fixing
here as well). I personally do not see the problem with this,
so honestly I do not see a reason to fix this.

Thanks!
-Lukas

> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/ext2/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 2885349..808f99f 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -927,7 +927,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
>  		}
>  		es = (struct ext2_super_block *) (((char *)bh->b_data) + offset);
>  		sbi->s_es = es;
> -		if (es->s_magic != cpu_to_le16(EXT2_SUPER_MAGIC)) {
> +		if (le16_to_cpu(es->s_magic) != EXT2_SUPER_MAGIC) {
>  			ext2_msg(sb, KERN_ERR, "error: magic mismatch");
>  			goto failed_mount;
>  		}
> 

Re: [PATCH] ext2: Make sb magic number endianness conversion consistent

[Carlos Maiolino]

On Thu, Oct 17, 2013 at 07:50:59PM +0200, Lukáš Czerner wrote:
> On Thu, 17 Oct 2013, Carlos Maiolino wrote:
> 
> > Date: Thu, 17 Oct 2013 14:24:58 -0300
> > From: Carlos Maiolino <cmaiolino@redhat.com>
> > To: linux-ext4@vger.kernel.org
> > Subject: [PATCH] ext2: Make sb magic number endianness conversion consistent
> > 
> > All ext2_super_block fields are converted from le16 to cpu when checked or
> > loaded into memory. Magic number check is the only field where we convert the
> > magic number to le16. So, make the conversion pattern consistent with the
> > remaining code
> 
> This not exactly true since this kind of comparison when a constant
> is converted rather the variable is present on several places in
> ext2, ext3 and ext4 (ext3 and ext4 have the same check you're fixing
> here as well). 

I agree here, it should have the same behavior on another extX versions, since
this code should have been copied from ext2 to another ones.

>I personally do not see the problem with this,
> so honestly I do not see a reason to fix this.
> 

No saying it's a problem, just a cosmetic stuff I figured out while doing a trip
on ext2 code. All of other comparisons (into the same funcion at least) converts
the SB value (which IMHO is the right thing to do since it's the data which
might have a different endianness).

Anyway, it just feel better to my eyes in this way, not that I have problems
with though

> Thanks!
> -Lukas
> 
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> >  fs/ext2/super.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> > index 2885349..808f99f 100644
> > --- a/fs/ext2/super.c
> > +++ b/fs/ext2/super.c
> > @@ -927,7 +927,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
> >  		}
> >  		es = (struct ext2_super_block *) (((char *)bh->b_data) + offset);
> >  		sbi->s_es = es;
> > -		if (es->s_magic != cpu_to_le16(EXT2_SUPER_MAGIC)) {
> > +		if (le16_to_cpu(es->s_magic) != EXT2_SUPER_MAGIC) {
> >  			ext2_msg(sb, KERN_ERR, "error: magic mismatch");
> >  			goto failed_mount;
> >  		}
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html