gmzang / maczfs

Automatically exported from code.google.com/p/maczfs
Other
0 stars 0 forks source link

IFDEFs around struct vnode/vnode_t #27

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
There's a few #ifdefs around function signatures that are essentially replacing 
vnode_t with struct 
vnode on the OSX port. I wonder why we can't typedef vnode_t to be struct 
vnode? Is this 
something that's defined elsewhere in OSX?

#ifdef __APPLE__
zfs_make_xattrdir(znode_t *zp, vattr_t *vap, struct vnode **xvpp, cred_t *cr)
#else
zfs_make_xattrdir(znode_t *zp, vattr_t *vap, vnode_t **xvpp, cred_t *cr)
#endif

http://github.com/alblue/mac-zfs/blob/master/usr/src/uts/common/fs/zfs/zfs_dir.c
#L876

Original issue reported on code.google.com by alex.ble...@gmail.com on 20 Feb 2010 at 3:29

GoogleCodeExporter commented 8 years ago
In the 10a286 code the 'struct vnode' way is replaced with the usual 'vnode_t'.

src/lib/libzpool/common/sys/zfs_context.h
/*
 * vnodes
 */
/* A subset of the kernel vnode structure. */ 
struct vnode {
    uint64_t    v_size;
    int     v_fd;
    char        *v_path;
};
#define vnode_t struct vnode

Original comment by jason.richard.mcneil on 20 Feb 2010 at 8:26

GoogleCodeExporter commented 8 years ago
Normally it would be defined with typedef struct vnode { ... } vnode_t; - 
however, I wonder if they have problems with that approach clobbering the type 
elsewhere. Also interesting that this only shows up here ...

Original comment by alex.ble...@gmail.com on 17 Jul 2010 at 9:45

GoogleCodeExporter commented 8 years ago
Oh, I see. vnode_t is usually defined as typedef struct vnode * vnode_t; in 
OSX, but in OpenSolaris, it's defined as typedef struct vnode vnode_t. So if we 
just did a straight typedef, we'd end up with the wrong thing, or at least 
inconsistent types. So by doing the #define trick, we avoid duping the type 
whilst giving it the right type for the OpenSolaris codebase to work. Neat.

http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/sys/vno
de.h

Original comment by alex.ble...@gmail.com on 17 Jul 2010 at 10:10

GoogleCodeExporter commented 8 years ago
OK, so the rabbit hole goes deeper than that. The problem is that (Apple's) 
mount.h, ubc. and file.h have references to the non-pointer stuff, so if we 
swap this #define in, it breaks. It looks like in the 10a286 bits they worked 
around this by forking zfs_file, zfs_ubc and zfs_mount which presumably stuck 
in the extra * in order to get things working again.

In other news, I'm pretty sure the zfs_context.h is unused for libzpool in the 
current layout. We really ought to get rid of unnecessary files.

Original comment by alex.ble...@gmail.com on 17 Jul 2010 at 10:34

GoogleCodeExporter commented 8 years ago
I've set the ground work in 
http://github.com/alblue/mac-zfs/commit/c8a4c5786fc0cd04f7aaa615c4a53d8ac53f5cd6
 and given it a brief spin. I still need to unpick the #ifdefs around before I 
can close this off, but it's a work in progress.

Original comment by alex.ble...@gmail.com on 18 Jul 2010 at 11:12

GoogleCodeExporter commented 8 years ago
Fixed in 
http://github.com/alblue/mac-zfs/commit/79d6126216e3cd8ee63126a23faa83b8e121111b

Original comment by alex.ble...@gmail.com on 18 Jul 2010 at 1:55