GNOME Bugzilla – Bug 136468
statfs() accepts 4 args on Solaris, IRIX
Last modified: 2004-12-22 21:47:04 UTC
1. [libgnomevfs/gnome-vfs-utils.c, configure.in] statfs() accepts 4 args on Solaris and IRIX. Test for this in configure.in. Solaris has the f_fstyp member, not f_ftype. Test for this as well. 2. [configure.in] Remove GNOME_PLATFORM_GNOME_2 and GNOME_PTHREAD_CHECK macros which are noops in GNOME 2.4. Missed AM_CONDITIONAL for HAVE_SSL.
Created attachment 25295 [details] [review] Patch
Added PATCH keyword and Priority to High to make it more visible for the maintainers.
This also applies to gnome-vfs 2.5, doesn't it ?
No idea. We're building for GNOME 2.4 and I presume gnome-vfs-2.5.x is for GNOME 2.6 which we haven't touched.
So this has been mostly fixed in HEAD (in the sun case), see bug #127193, but your patch looks more generic, so I'd tend use yours instead of what was applied.
After rereading the bug previously mentioned, Solaris seems to have statvfs so #if HAVE_STATVFS statfs_result = statvfs (unescaped_path, &statfs_buffer); #else - statfs_result = statfs (unescaped_path, &statfs_buffer); +#if STATFS_ARGS == 2 + statfs_result = statfs (unescaped_path, &statfs_buffer); +#elif STATFS_ARGS == 4 + statfs_result = statfs (unescaped_path, &statfs_buffer, + sizeof (statfs_buffer), 0); +#endif #endif shouldn't be an issue on Solaris. Did you add that because of Irix ? For the rest of the patch, I'd tend to do as in HEAD, ie put stuff around #if defined (__linux__) and #if defined (__sun) since it's pretty unlikely that ncpfs exists on something else.
The patch which was applied to head: http://bugzilla.gnome.org/showattachment.cgi?attach_id=21825
IRIX takes a 4-arg statfs(). The ncpfs stuff is most likely Linux-specific. However, it should fail everywhere else. I really dislike special-casing ala #ifdef(__sun) or #ifdef(__linux) unless it is absolutely necessary. Is it really necessary in this case? The code looks much cleaner without it. And, this part of the patch is icky: +#elif defined(__sun) + if (strcmp(statfs_buffer.f_basetype, "ncpfs") == 0) +#endif We should AC_CHECK_MEMBERS(struct statfs.f_basetype) and #ifdef based on the result. That's the autoconf way.
If we consider ncpfs being linux specific, it makes no sense to make the part of the code which special cases ncpfs filesystems portable. With and #ifdef __linux__, it's immediately obvious that this code was meant to be linux only.
Ok.
Created attachment 25358 [details] [review] proposed patch
How does that patch look ? Is it working for you ?
Yep, that's ok.
I committed this patch to the 2.4 branch, it's included in gnome-vfs 2.4.3 which was just released. I'm attaching a patch against HEAD to this bug, it corresponds to the changes I made in the 2.4 branch, but gnome-vfs is currently frozen for the 2.6.0 so it will be committed after it is released.
Created attachment 25408 [details] [review] sync head with the 2.4 branch
This can go in on HEAD now, since we branched. Maybe we should get it into 2.6.1 too?
Checked in on HEAD and gnome-2-6