Looks good to me. My only suggestions would be to rename it to PVFS2_ALIGN_VAR and put it in pvfs2-types.h in case we want to use it elsewhere. Maybe put PVFS2_ on the SIZEOF_VOIDP too, to make sure it doesn't ever conflict with something that autoconf generates.

-Phil


Sam Lang wrote:

Except it needs to be like this for the kernel module:

#if defined(__WORDSIZE)
  #define SIZEOF_VOIDP __WORDSIZE
#elif defined(BITS_PER_LONG)
  #define SIZEOF_VOIDP BITS_PER_LONG
#elif defined(INTPTR_MIN)
  #if   INTPTR_MIN == INT32_MIN
    #define SIZEOF_VOIDP 32
  #elif INTPTR_MIN == INT64_MIN
    #define SIZEOF_VOIDP 64
  #endif
#else
  #error "Unhandled size of void pointer"
#endif


On Apr 5, 2006, at 12:56 PM, Sam Lang wrote:


What about this? It makes pvfs2-sysint.h a bit messier at the top of the file, but the struct definition is cleaner.

-sam

#if defined(__WORDSIZE)
#define SIZEOF_VOIDP __WORDSIZE
#elif defined(INTPTR_MIN)
#if INTPTR_MIN == INT32_MIN
#define SIZEOF_VOIDP 32
#elif INTPTR_MIN == INT64_MIN
#define SIZEOF_VOIDP 64
#endif
#else
#error "Unhandled size of void pointer"
#endif

#if SIZEOF_VOIDP == 32
#define ALIGN_VAR(_type, _name) \
    _type _name; \
    int32_t __pad##_name
#else
#define ALIGN_VAR(_type, _name) _type _name
#endif

/** Holds a non-blocking system interface operation handle. */
typedef PVFS_id_gen_t PVFS_sys_op_id;

/** Describes attributes for a file, directory, or symlink. */
struct PVFS_sys_attr_s
{
    PVFS_uid owner;
    PVFS_gid group;
    ALIGN_VAR(PVFS_permissions, perms);
    PVFS_time atime;
    PVFS_time mtime;
    PVFS_time ctime;
    PVFS_size size;
    ALIGN_VAR(char *, link_target); /* NOTE: caller must free this */
ALIGN_VAR(int32_t, dfile_count); /* Changed to int32_t so that size of structure does not change */
    PVFS_size dirent_count;
    PVFS_ds_type objtype;
    uint32_t mask;
};

On Apr 5, 2006, at 12:28 PM, Sam Lang wrote:


We add -Wundef with --enable-strict, but I configured my build with --enable-strict and didn't see the warning. It looks like the kernel module code only gets the flags specified when the kernel was built? Once I added the -Wundef to CFLAGS in my kernel headers Makefile, I was able to reproduce the warnings while building the kernel module. Now that I can reproduce it, I'll try to fix it up so that we have something that works in both cases.

-sam

On Apr 5, 2006, at 11:48 AM, Phil Carns wrote:

Ahh, -Wundef is the difference. That flag is getting set automatically on my laptop for some reason but not on the other boxes that I try.

-Phil

Phil Carns wrote:

I'll see if I can figure out something to do based on Nathan's suggestion. Looking a little closer today, it looks like maybe there is more of a compiler difference (or -W flag difference) that determines if you see the warning, rather than a kernel version? For example, on RHEL4 (x86_64) I do not see the warning. However, if I add this to the pvfs2-sysint.h header:
#ifdef __KERNEL__
    #ifndef INTPTR_MIN
    #error unable to find INTPTR_MIN
    #endif
#endif
... then it does not compile. So maybe the #if checks don't always generate a warning if some of the defines are missing? That's kind of disappointing if so, because it means we have the opportunity to silently get the size wrong at build time. I guess the
"#if INTPTR_MIN == INT32_MIN"
is always getting processed as true if those values are undefined.
-Phil
Sam Lang wrote:


Hi Phil,

I'm not able to reproduce that warning with a 2.6.11 kernel. I'm not sure why as the limits.h that gets included doesn't seem to define INTPTR_MIN either. Could you just send me a patch with Nathan's proposed fix or something similar that gets rid of those warnings for you?

Thanks,

-sam

On Apr 4, 2006, at 3:30 PM, Phil Carns wrote:

Actually, it does compile now that I look closer (I had a second problem confusing me), but it does generate quite a few warnings like this:

pvfs2/include/pvfs2-sysint.h:44:5: warning: "INTPTR_MIN" is not defined pvfs2/include/pvfs2-sysint.h:44:19: warning: "INT32_MIN" is not defined

Not sure what the ramifications are of the warning, though.

-Phil

Phil Carns wrote:

Is INTPTR_MIN defined in the kernel headers somewhere as well? I am having a hard time compiling the kerne module at the moment because /kernel/linux-2.6/pvfs2-utils.c ends up pulling in pvfs2- sysint.h. I am using the 2.6.15.4 kernel.
-Phil
Sam Lang wrote:


This seems to work everywhere we tried, so I went ahead and committed that change. Thanks Pete!

-sam

On Apr 4, 2006, at 12:15 PM, Pete Wyckoff wrote:

[EMAIL PROTECTED] wrote on Tue, 04 Apr 2006 11:32 -0500:

Hm...actually I didn't notice before but the use of __WORDSIZE or BITS_PER_LONG will break on darwin (which doesn't define either).
Previously, I fixed this by defining SIZEOF_LONG_INT and
SIZEOF_VOID_P in pvfs2-config.h, but the PVFS_sys_attr_s struct now includes padding and that struct is defined in pvfs2- sysint.h, which
is an external header.






Posix defines intptr_t as an int that can hold a pointer, hence you can get your word size out of that. Then this should work and uses
only constants that POSIX requires in <stdint.h>:

    #if INTPTR_MIN == INT32_MIN
    32-bit
    #else
    not 32-bit
    #endif

Appears visually to be okay on darwin, x86, and x86_64. But I've never tried it in real life. Too bad there appears to be no direct
wordsize in posix anywhere.


Another approach would be with unions:

    struct PVFS_sys_attr_s {
    ...
    union {
        char _s[8];
        char *link_target;
    };
    ...
    }

But I haven't tried that either. On ancient gcc you need to give
the union a name then "#define link_target _u.link_target",  e.g.

            -- Pete


_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2- developers



_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2- developers





_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2- developers






_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers

Reply via email to