xfs
[Top] [All Lists]

Re: [PATCH 04/14] xfsprogs: prefix XATTR_LIST_MAX with XFS_

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 04/14] xfsprogs: prefix XATTR_LIST_MAX with XFS_
From: Jan Tulak <jtulak@xxxxxxxxxx>
Date: Thu, 24 Sep 2015 10:28:05 +0200
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150923031532.GN3902@dastard>
References: <1442311164-12921-1-git-send-email-jtulak@xxxxxxxxxx> <1442311164-12921-5-git-send-email-jtulak@xxxxxxxxxx> <20150923031532.GN3902@dastard>
On Wed, Sep 23, 2015 at 5:15 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
On Tue, Sep 15, 2015 at 11:59:14AM +0200, Jan Tulak wrote:
> WILL CHANGE THE COMMIT MESSAGE.

OK?Â
> All right, I make the renaming with define - though I'm not sure
> that with the ifdef for OS X and SIZE_MAX moved to a standalone patch
> we need it - shouldn't be this change rather dropped?
>
> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
> ---
> include/xfs.h   | 2 ++
>Â libhandle/handle.c | 4 ++--
> libhandle/jdm.c  | 4 ++--
>Â 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/xfs.h b/include/xfs.h
> index bc94068..8ee0106 100644
> --- a/include/xfs.h
> +++ b/include/xfs.h
> @@ -53,6 +53,8 @@
>Â # define ASSERT(EX)Â ((void) 0)
>Â #endif
>
> +#define XFS_XATTR_LIST_MAX XATTR_LIST_MAX
> +

This does not belong here - it is a limit that applies to the ioctl
API and so must be the same in userspace and the kernel. Such
definitions belong in libxfs/xfs_fs.h, and should respect local OS
limits if defined. e.g. something like:

#ifdef XATTR_LIST_MAX
#define XFS_XATTR_LIST_MAX XATTR_LIST_MAX
#else
#define XFS_XATTR_LIST_MAX 65536
#endif


âOK, âthat makes sense.

Â
Will work on both the kernel and userspace side. This will also need
a kernel side patch...

âNow, this is something where I may need a bit help/confirmation, I guess.â
What I suppose I should do is:

2) EditÂlinux-xfs/fs/xfs/libxfs/xfs_fs.h and add the same change there
3) Send it to this mailing list as usual, with "[PATCH] xfs: ..."

(The same goes for the other patch with the kernel need too.)

Is it all right? Correct repository URL? :-)

Thanks,
Jan

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