xfs
[Top] [All Lists]

Re: [PATCH 08/16] xfs: change interface of xfs_nameops.hashname

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 08/16] xfs: change interface of xfs_nameops.hashname
From: Ben Myers <bpm@xxxxxxx>
Date: Tue, 14 Oct 2014 10:34:46 -0500
Cc: linux-fsdevel@xxxxxxxxxxxxxxx, olaf@xxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20141006221708.GI2301@dastard>
References: <20141003214758.GY1865@xxxxxxx> <20141003215844.GG1865@xxxxxxx> <20141006221708.GI2301@dastard>
User-agent: Mutt/1.5.20 (2009-06-14)
On Tue, Oct 07, 2014 at 09:17:08AM +1100, Dave Chinner wrote:
> On Fri, Oct 03, 2014 at 04:58:44PM -0500, Ben Myers wrote:
> > From: Olaf Weber <olaf@xxxxxxx>
> > 
> > With the introduction of the xfs_nameops.normhash callout, all uses of the
> > hashname callout now occur in places where an xfs_name structure must be
> > explicitly created just to match the parameter passing convention of this
> > callout. Change the arguments to a const unsigned char * and int instead.
> > 
> > Signed-off-by: Olaf Weber <olaf@xxxxxxx>
> > 
> > [v2: pass a 3rd argument for sb_utf8version to hashname.  --bpm]
> 
> So now I've looked at most of the rest of the patch set, I think
> this is the wrong thing to do. I see no reason apart from "it's less
> typing" to drop the use of the xfs-name structure, but it removes a
> key piece of documentation from the code. i.e. that the name/namelen
> are an inseparable tuple and cannot be separated. Indeed, lots of
> the utf8 xfs code declares norm/normlen tuples on the stack for
> temporary use, so really this comes down to a matter of taste.
> 
> And in that matter, I'd prefer that we keep the existing name
> abstaction and propagate it into the new code rather than the other
> way around.

Does something like this suit you?

struct xfs_name {
        const unsigned char     *name;
        int                     len;
        int                     type;
        __uint32_t              utf8version;
};

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