xfs
[Top] [All Lists]

RE: [PATCH] xfsprogs: fix unaligned access in libxfs

To: "Christoph Hellwig" <hch@xxxxxxxxxxxxx>
Subject: RE: [PATCH] xfsprogs: fix unaligned access in libxfs
From: "Alex Elder" <aelder@xxxxxxx>
Date: Fri, 28 Aug 2009 16:39:49 -0500
Cc: "Gabriel Vlasiu" <gabrielvlasiu@xxxxxxxxx>, <xfs@xxxxxxxxxxx>
In-reply-to: <20090828153718.GA26409@xxxxxxxxxxxxx>
Thread-index: Acon+mrbCI249/EMQiKjyaA3Ujst/AALHnGg
Thread-topic: [PATCH] xfsprogs: fix unaligned access in libxfs
Christoph Hellwig wrote:
> The get/put unaligned handlers we use to access the extent descriptor
> are not good enough for architectures like Sparc that do not tolerate
> dereferencing unaligned pointers.  Replace the implementation with the
> one the kernel uses on these architectures.  It might be a tad
> slower on architectures like x86, but I don't want to have multiple
> implementations around to not let the testing matrix explode.
> Also remove the unaligned.h header which includes another implementation
> for unaligned access we don't actually use anymore.
> 
> Note that the little change to xfs_inode.c needs to go into the kernel
> aswell, I will send a patch for that shortly.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reported-by: Gabriel Vlasiu <gabrielvlasiu@xxxxxxxxx>
> Tested-by: Gabriel Vlasiu <gabrielvlasiu@xxxxxxxxx>

Looks good.  Note comment about interface below.

Reviewed-by: Alex Elder <aelder@xxxxxxx>


> Index: xfsprogs-dev/libxfs/xfs.h
> ===================================================================
> --- xfsprogs-dev.orig/libxfs/xfs.h    2009-08-27 10:06:13.377855006 -0300
> +++ xfsprogs-dev/libxfs/xfs.h 2009-08-27 10:06:26.537884492 -0300
> @@ -127,19 +127,37 @@ static inline int __do_div(unsigned long
>  #define max_t(type,x,y) \
>       ({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
> 
> -/* only 64 bit accesses used in xfs kernel code */
> -static inline __u64 get_unaligned_be64(void *ptr)
> +
> +static inline __uint32_t __get_unaligned_be32(const __uint8_t *p)


It would be nice for this interface (and others) to take a (void *)
argument, but that would also mean immediately casting it to do
pointer arithmetic portably.  Not a big deal.

> +{
> +        return p[0] << 24 | p[1] << 16 | p[2] << 8 | p[3];
> +}
> +
> +static inline __uint64_t get_unaligned_be64(void *p)
>  {

. . .

<Prev in Thread] Current Thread [Next in Thread>