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

["Barry Naujok" <bnaujok@xxxxxxx>]

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!