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 17:38:29 -0500
Cc: "Gabriel Vlasiu" <gabrielvlasiu@xxxxxxxxx>, <xfs@xxxxxxxxxxx>
In-reply-to: <20090828221530.GB17666@xxxxxxxxxxxxx>
Thread-index: AcooLQ7AdYGD0OqKRbaJx+exmoC3IQAAnNpA
Thread-topic: [PATCH] xfsprogs: fix unaligned access in libxfs
Christoph Hellwig wrote:
> On Fri, Aug 28, 2009 at 04:39:49PM -0500, Alex Elder wrote:
>>> +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.
> 
> the __ routines are internal implementation details, the kernel
> has non-__ prefixed versions that take void pointers, but I didn't
> bother to add them to xfsprogs as we only do unaligned access to
> 64bit values.
> 
> That beeing said I dislike the interface taking void pointers as
> it removes important error checking.  The actually visible interface
> should take __be16/32/64 types to enforce they are used on the right
> type.  Once I get some time I will push for that in the kernel again,
> but for xfsprogs I really do want to stick to the kernel interfaces so
> that we can share the code unmodified.

The results should be int types:  __be16, etc., I agree.  But
I'm saying the unaligned addresses--which we (now) make no
assumptions about and treat as character pointers--I think
the void pointer explicitly captures that unaligned quality
(and avoids any need for a cast).

In any event I think it's fine either way.  Really not a big
deal, just a tiny detail that only a geek could love to
argue about.

                                        -Alex

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