[EMAIL PROTECTED] wrote on Thu, 30 Mar 2006 12:30 -0600:
> Anyways, I would love to hear comments or suggestions on the patch.
> - char *link_target; /* NOTE: caller must free if valid */
> - int dfile_count;
> + uint64_t link_target; /* NOTE: caller must free if valid. Needs to be
> uint64_t so that it is a fixed length entity */
> + int32_t dfile_count;
This is by far the most objectionable component, along with all the
string casts. Could you use #ifdef on word size to avoid this?
> struct PVFS_sys_mntent
> {
> char **pvfs_config_servers; /* addresses of servers with config
> info */
> - int num_pvfs_config_servers;
> + int32_t num_pvfs_config_servers;
Are you saying int is 64-bit on ppc64? That would be very odd indeed.
I find the need to encode the type for native ints quite intrusive. Is
this really the way these things are handled?
> + tmp_cflags=$CFLAGS
> + CFLAGS="$CFLAGS"
What good does this do?
> - size_t amt_complete;
> + int64_t amt_complete;
size_t is unsigned, int64_t is signed.
> - loff_t readahead_size;
> + int32_t readahead_size;
> } pvfs2_io_request_t;
Lost precision here, 64->32. Do possible values still fit?
> + {
> + int ret = 0;
> ret = copy_from_user(
No need to initialize to zero if you're immediatly giving it a value
(three places).
In spite of the little comments above, I think this is important to
do and good stuff. But testing on all-64 and all-32 arches has to
happen to watch out for conversion bugs.
-- Pete
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
