GNOME Bugzilla – Bug 387200
libgtop fails to build or work on FreeBSD
Last modified: 2007-02-05 21:51:50 UTC
Downstream bug http://bugs.gentoo.org/show_bug.cgi?id=158030 (2.15 also fails, but we don't have that in portage yet) Patches to follow
Created attachment 78574 [details] [review] Fix generic read/write
Created attachment 78575 [details] [review] FreeBSD patch Make libgtop work with Gentoo/FreeBSD-6 This patch is made up from patches taken from the FreeBSD ports tree - I couldn't find any reference to this upstream so I'm basically submitting their patches. I cannot claim to understand it completely, but it does enable System Monitor to report everything correctly.
Created attachment 80596 [details] [review] FreeBSD patch For 2.14.6
Sorry i'm late. Let me first hand your patch to Petr Salinger so he can check if it's OK with kfreebsd.
(In reply to comment #4) > Sorry i'm late. Let me first hand your patch to Petr Salinger so he can check > if it's OK with kfreebsd. At first look, it will break GNU/kFreeBSD, moreover it is wrong: + int result; + struct statfs sfs; +#if __FreeBSD_version >= 600000 + struct devstat *ds; + void *sc; + struct timespec ts; + struct gprovider *gp; + struct gident *gid; + struct gmesh gmp; + double etime; + uint64_t ld[2]; #endif + + if (result == -1) { + glibtop_warn_io_r (server, "statfs"); + return; So the "result" is autovariable, it contains garbage from stack, but is is compared to "-1". In general, I would be nice to have some common header included from all (k)freebsd specific sources, which would contain #include <sys/param.h> #if !defined(__FreeBSD_kernel_version) && defined(__FreeBSD_version) #define __FreeBSD_kernel_version __FreeBSD_version #endif #if !defined(__FreeBSD_kernel__) && defined(__FreeBSD__) #define __FreeBSD_kernel__ __FreeBSD__ #endif And all tests against kernel version would use only __FreeBSD_kernel_version/ __FreeBSD_kernel__. The macro __FreeBSD__ signals FreeBSD kernel and FreeBSD libc, the macro __FreeBSD_kernel__ signals FreeBSD kernel. So i.e. access for <machine/endian.h> should be guarded by __FreeBSD__, but in most cases in libgtop it should be guarded by __FreeBSD_kernel__/__FreeBSD_kernel_version. Petr
Created attachment 81448 [details] [review] FreeBSD patch OK, I've taken the above comments into account. This new patch should address them. NOTE - Existing #ifdefs for any fbsd stuff have not been touched.
The last version of patch does not break GNU/kFreeBSD, but due to dropping POSIX <sys/statvfs.h> it still might break other OS which use freebsd sysdeps (netbsd*|openbsd*|bsdi*). The suggestion for common header have been meant mainly for future, not for 2.14.x series. Would be possible for 2.15 or 2.17 series rename sysdeps/freebsd/ into sysdeps/bsd/ and apply the idea of common header included from all bsd specific sources and change in all tests __FreeBSD__ -> __FreeBSD_kernel__ __FreeBSD_version -> FreeBSD_kernel_version I would after that test on GNU/kFreeBSD and mark places, where really macros __FreeBSD__ and __FreeBSD_version belongs. Petr
(In reply to comment #7) > The last version of patch does not break GNU/kFreeBSD, > but due to dropping POSIX <sys/statvfs.h> it still might break > other OS which use freebsd sysdeps (netbsd*|openbsd*|bsdi*). I checked NetBSD man pages and they no longer define statfs. As I said earlier, the patch contents are from FreeBSD ports and I haven't had time to fully understand them yet. I'll see if I can make it work using statvfs - shouldn't be hard though. > The suggestion for common header have been meant mainly for future, > not for 2.14.x series. Fair enough. I'm just interested in the here and now though. So will that code work on kFreeBSD using libgeom? Should I be testing __FreeBSD_version and FreeBSD_kernel_version at the same time? Thanks Roy
> I'm just interested in the here and now though. So will that > code work on kFreeBSD using libgeom? Should I be testing __FreeBSD_version > and FreeBSD_kernel_version at the same time? No. We have libgeom, but we do not have (yet ?) libdevstat. Petr
Created attachment 81508 [details] [review] FreeBSD patch Lets make things very simple then. It seems the error is purely because FreeBSD statvfs does not define the required fields in the structure. So simply undefing the fact that it has them enables it to work. This should also enable accurate reporting on kFreeBSD - but I cannot test this.
Looks good for GNU/kFreeBSD. Only in sysdeps/freebsd/proctime.c chunk "#if (__FreeBSD_version >= 600000) || (__FreeBSD_kernel_version >= 600000)" could be used instead of "#if __FreeBSD_version >= 600000". Petr
Created attachment 81514 [details] [review] FreeBSD patch Good catch. Hopefully this is now final and can be merged :)
Thank you both. Is the bug about read/write still relevant ?
(In reply to comment #13) > Thank you both. Is the bug about read/write still relevant ? Not any more from my testing. If it rears it's ugly head again, I'll open a new bug if needed :)
Closing then. Thanks again.