GNOME Bugzilla – Bug 125049
libgtop should provide block size with fs-usage
Last modified: 2004-12-22 21:47:04 UTC
Currently, the libgtop API only provides: struct _glibtop_fsusage { u_int64_t flags, blocks, /* Total blocks. */ bfree, /* Free blocks available to superuser. */ bavail, /* Free blocks available to non-superuser. */ files, /* Total file nodes. */ ffree; /* Free file nodes. */ } But the block number is pretty useless without the size of each block! For instance, on my laptop gnome-system-monitor incorrectly reports a 10 Gb disk when the true size is 20 Gb disk because it assumes the block size to be 512. Unfortunately, there seems to be different ways of retrieving the block size on different OSes. That's why it really should be in libgtop. I have some code for my Hardware Monitor applet: #if HAVE_SYS_STATFS_H #include <sys/statfs.h> #endif [...] disk_block_size = 512; // 512 is apparently pretty common #if HAVE_SYS_STATFS_H // with statfs we can, however, do better struct statfs buf; if (statfs(mount_dir.c_str(), &buf) == 0) disk_block_size = buf.f_bsize; #endif With the following in configure.ac: dnl for getting the block size AC_CHECK_HEADER(sys/statfs.h) This works for GNU/Linux as least for a start (IIRC for Solaris too). I think there is a much more complicated, portable version in the sources for 'df' in GNU Coreutils. Augmenting the fsusage structure with a block_size field should be easy.
*** Bug 125060 has been marked as a duplicate of this bug. ***
libgtop just get's it from /proc/meminfo on linux. I'm not sure why that would be wrong. Can you check that /proc/meminfo is incorrect?
I am completely mystified. /proc/meminfo is about swap and memory info, right? I am talking about disk usage. Disk as in disk, not disk as in swap. :-) I think you need to reopen bug #125060. I will post the line numbers for the wrong code in gnome-system-monitor in there.
woops! Sorry about that ;)
Ole, could you cook up a patch? This would have better chance of actually getting in here.
I've looked into the issue. There was actually some code (stolen from an old release of GNU Coreutils) to ensure that the reported no. of blocks was normalised to a block size of 512 bytes, but apparently it didn't work. I also don't understand why one would want to support the notion of blocks if the block size is completely artificial. So I've made a patch that syncs with the latest version of the code from Coreutils (in a less intrusive manner so it should be easy to resync if necessary), avoids the normalisation step and instead adds the field block_size to the fsusage struct. I've tested it by modifying one of the example programs. It seems to work - in fact it is simply the code that is used for 'df' so it must work. :-) It is important that it be mentioned in the release notes that this change has occurred, because it most likely _will_ break any program that assumes the block size to be 512 bytes. They will go from mostly-working to mostly-not-working. I'm attaching a tarball with a patch and two extra files (I don't know how to get them into the patch). I do have CVS access so I can easily commit the changes if you like them. BTW, I had to add AM_PROG_AS to configure.in to get the build system to work with Debian's versions of automake/autoconf. PS: Thanks for looking after an otherwise dusty old module!
Created attachment 22735 [details] Tarball (tar.gz) with patch and two extra files
Patch is in! Whee!
Top-notch patch Ole. Added to CVS HEAD now. 2003-12-27 Ole Laursen <olau@hardworking.dk> * include/glibtop/fsusage.h: Added block_size field. (Closes: #125049)
the size of a block is currently defined as bellow int block_size; libgtop is now using the glib, so whe should use glib integer types, as every other libgtop struct do (most of it use guint64 and guint32. i think block_size should stored in a guint16 because large enough and glib compliant
same problem with uintmax_t sysdeps/common/fsusage.[ch] and sysdeps/common/fsusage-frontend.c
and GLIBTOP_MAX_FSUSAGE hasn't been updated +#define GLIBTOP_FSUSAGE_BLOCK_SIZE 6 #define GLIBTOP_MAX_FSUSAGE 5 who has authorized the following changes ? - guint64 fsu_blocks; /* Total blocks. */ - guint64 fsu_bfree; /* Free blocks available to superuser. */ - guint64 fsu_bavail; /* Free blocks available to non-superuser. */ - guint64 fsu_files; /* Total file nodes. */ - guint64 fsu_ffree; /* Free file nodes. */ + int fsu_blocksize; /* Size of a block. */ + uintmax_t fsu_blocks; /* Total blocks. */ + uintmax_t fsu_bfree; /* Free blocks available to superuser. */ + uintmax_t fsu_bavail; /* Free blocks available to non-superuser. */ + int fsu_bavail_top_bit_set; /* 1 if fsu_bavail represents a value < 0. */ + uintmax_t fsu_files; /* Total file nodes. */ + uintmax_t fsu_ffree; /* Free file nodes. */ nobody has reviewed this patch ?
Created attachment 25626 [details] [review] possible fix
int and gint are the same thing, so we should just change the external interfaces to use gint. uintmax_t is pretty crappy indeed, we'll use guint64. I guess that Ole's patch was against an older version of libgtop, and that before 2.5.x, we used uintmax_t and other stuff like that from stdint.h...
2004-03-15 Bastien Nocera <hadess@hadess.net> * include/glibtop/fsusage.h: set GLIBTOP_MAX_FSUSAGE properly, use gint in the headers instead of int (Closes: #125049) 2004-03-15 Bastien Nocera <hadess@hadess.net> * fsusage.c: * fsusage.h: remove use of uintmax_t
i think your patch is incomplete - fsusage.c still uses quite obfuscated macros with uintmax_t then some clean up is needed: - #include <stdint.h>, <inttypes.h>, etc are now unused and should be removed - struct fs_usage::fsu_bavail_top_bit_set might be a gboolean instead of int - libgtop use 2 redundant data types : glibtop_fsusage and struct fs_usage
Perhaps I can clarify. I stole the code in fsusage.[ch] from GNU coreutils, and as explained in README.fsusage I left them almost unchanged and instead wrote a frontend to make it easier to sync the code later on if needed. The previous (buggy) version of this code was modified more to fit into libgtop and was not later resynced, which I think is the cause of the bug. But it is of course your decision.
Benoit, the code wasn't committed because of problems with the CVS server. You could at least check that the ChangeLog entries are present in the version you're testing, that's why I paste them in the bugs...
It's been committed now. As Ole explained, we need to keep the changes to fsusage.[ch] to a minimum. I think that removing the use of uintmax_t et al is enough, and not too much work on the maintainers' side.
*** Bug 138918 has been marked as a duplicate of this bug. ***
Created attachment 27427 [details] [review] Missing block_size name
Comment on attachment 27427 [details] [review] Missing block_size name oop,s i don't know why block_size is gint, GLIBTOP_TYPE_ULONG -> GLIBTOP_TYPE_INT
Created attachment 27535 [details] [review] added name for fs_usafe.block_size