Re: [PATCH-mm 1/8] ntfs: remove private wrapper around endian helpers

[Anton Altaparmakov]

Hi,

On 27 Nov 2008, at 19:35, Harvey Harrison wrote:
> On Thu, 2008-11-27 at 08:22 +0000, Anton Altaparmakov wrote:
> > On 26 Nov 2008, at 21:12, Harvey Harrison wrote:
> > > The base versions handle constant folding just fine.
> > > Done with sed other than the mft.c and inode.c one-liners.
> >
> > Have you checked the compiler output?  And on all architectures?  And
> > on all gcc versions?
>
> Yes I've checked the compiler output for the new byteorder helpers
> for gcc 3.4, 4, 4.2 on x86.
>
> Notice this depends on the byteorder bits in -mm.  The failure of
> the old bits to properly compile-time swap I've only seen at -O0,
> which the new bits do properly.  If you can cite a particular
> version/arch that you know of where the bug arose, please let me
> know.

Ah, I failed to notice this required some new byteorder code!  Sorry.

> > The reason I introduced the constant version are precisely that at
> > least some versions of gcc at least on some architectures do NOT
> > handle the constant folding and actually put in endianness conversion
> > code for the constants instead of converting them at compile time.
>
> Which ones....-O0 builds perhaps?
>
> > Also you are breaking sparse checking with this patch as you remove
> > some of the necessary endianness conversions (e.g. your one liner in
> > mft.c which causes us to apply a binary 'not' operator on a little
> > endian which sparse complains about - in fact that is the reason the
> > endianness conversions are in there in the first place - I added them
> > because sparse was complaining about them).
>
> No, sparse does not complain about it.  Bitwise operations are just
> fine on types such as le16 etc.  Try running sparse again with
> my changes, there are no new warnings.

Interesting.  Sparse must have grown up then.  It used to complain  
(although I always thought it was complaining incorrectly given that  
binary bit operations are endianness agnostic).

> > Andrew, please do not apply this patch.

> I hope you change you mind.

Yes, you have convinced me.  (-:

Andrew, please feel free to apply the patch.

Best regards,

	Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

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