xfs
[Top] [All Lists]

Re: [PATCH 1/7] XFS: Name operation vector for hash and compare

To: "Josef 'Jeff' Sipek" <jeffpc@xxxxxxxxxxxxxx>
Subject: Re: [PATCH 1/7] XFS: Name operation vector for hash and compare
From: "Barry Naujok" <bnaujok@xxxxxxx>
Date: Thu, 03 Apr 2008 14:50:39 +1000
Cc: xfs@xxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx
In-reply-to: <20080403002246.GB5211@xxxxxxxxxxxxxx>
Organization: SGI
References: <20080402062508.017738664@xxxxxxxxxxxxxxxxxxxxxxx> <20080402062707.797672682@xxxxxxxxxxxxxxxxxxxxxxx> <20080403002246.GB5211@xxxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Opera Mail/9.24 (Win32)
On Thu, 03 Apr 2008 10:22:46 +1000, Josef 'Jeff' Sipek <jeffpc@xxxxxxxxxxxxxx> wrote:

On Wed, Apr 02, 2008 at 04:25:09PM +1000, Barry Naujok wrote:
...
+/*
+ * Name ops for directory and/or attr name operations
+ */
+
+typedef xfs_dahash_t   (*xfs_hashname_t)(const uchar_t *, int);
+typedef xfs_dacmp_t    (*xfs_compname_t)(const uchar_t *, int,
+                                         const uchar_t *, int);

Why have typedefs for function pointers? Sometimes, they even cause problems
(I remember Eric finding a nasty 64-bit bug related to a function pointer
typedef).

Since IRIX isn't on the supported OS list anymore, what's the policy with
coding style within XFS?

Ok, I have fixed it:

+/*
+ * Name ops for directory and/or attr name operations
+ */
+struct xfs_nameops {
+       xfs_dahash_t (*hashname)(const uchar_t *, int);
+       xfs_dacmp_t  (*compname)(const uchar_t *, int, const uchar_t *, int);
+};

...
Index: kern_ci/fs/xfs/xfs_dir2.h
===================================================================
--- kern_ci.orig/fs/xfs/xfs_dir2.h
+++ kern_ci/fs/xfs/xfs_dir2.h
@@ -85,6 +85,12 @@ extern int xfs_dir_canenter(struct xfs_t
                                char *name, int namelen);
 extern int xfs_dir_ino_validate(struct xfs_mount *mp, xfs_ino_t ino);

+#define xfs_dir_hashname(dp, n, l) \
+               ((dp)->i_mount->m_dirnameops->hashname((n), (l)))
+
+#define xfs_dir_compname(dp, n1, l1, n2, l2) \
+               ((dp)->i_mount->m_dirnameops->compname((n1), (l1), (n2), (l2)))

#define vs. static inline...

I guess this comes back to my question before...what is the coding style
direction you want XFS to go in? More Linux-like (static inline)? or keep it
more IRIX-like (#define)?

Nasty gotcha in this scenario, I have added a comment before them:

+/*
+ * Macros are used calling for the xfs_inode's xfs_mount's name operations as + * in most cases, xfs_dir2.h is included before xfs_inode.h and xfs_mount.h.
+ */
+#define xfs_dir_hashname(dp, n, l) \
+               ((dp)->i_mount->m_dirnameops->hashname((n), (l)))
+
+#define xfs_dir_compname(dp, n1, l1, n2, l2) \
+               ((dp)->i_mount->m_dirnameops->compname((n1), (l1), (n2), (l2)))

The alternative is reorganising the #includes in most of the .c files!




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