Re: [PATCH] corrupted free inodes list
On Fri, 22 Dec 2006, Thomas Dickey wrote: > On Fri, 22 Dec 2006, Leonard den Ottolander wrote: > >> Hi Mikulas, >> >> On Wed, 2006-12-20 at 02:15 +0100, Mikulas Patocka wrote: >>> --- maybe you could change double to "long long" but I'm not sure if it >>> exists on all machines --- a configure test would probably be needed. >> >> Yes. Using floats for discrete counters is not such a good idea. > > It's likely to be slower, but if you don't exceed the precision of the > mantissa, you won't lose information. > > The general reader, taking into account the comment about "discrete" is > likely to assume that you meant information would be lost. Since that's > not the case (for the assumed 48 bits), it's worth clarifying the comment. I am working to get this fixed. I plan to use code from coreutils's df to calculate the percent. I also resurrected the files fsusage.[ch] where originally the get_fs_usage() function resided. fsusage.[ch] come from gnulib/coreutils/filutils and its maintainers had switched to uintmax_t to collect the disk usage statistic for a long time now. I'll let you know when I am done. If someone is willing to help take a look at show_dev() in coreutils's source code. I am also wondering whether we won't to import human.[ch] from gnulib. IMO, it makes sense, but still this code does bit more than what we need but maybe in the long term its worth to rely on a known good and supported code instead of a homebrew solution. Please, let me know what do you think. Thanks! ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [PATCH] corrupted free inodes list
On Fri, 22 Dec 2006, Leonard den Ottolander wrote: > Hi Mikulas, > > On Wed, 2006-12-20 at 02:15 +0100, Mikulas Patocka wrote: >> --- maybe you could change double to "long long" but I'm not sure if it >> exists on all machines --- a configure test would probably be needed. > > Yes. Using floats for discrete counters is not such a good idea. It's likely to be slower, but if you don't exceed the precision of the mantissa, you won't lose information. The general reader, taking into account the comment about "discrete" is likely to assume that you meant information would be lost. Since that's not the case (for the assumed 48 bits), it's worth clarifying the comment. -- Thomas E. Dickey http://invisible-island.net ftp://invisible-island.net ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [PATCH] corrupted free inodes list
Hi Mikulas, On Wed, 2006-12-20 at 02:15 +0100, Mikulas Patocka wrote: > --- maybe you could change double to "long long" but I'm not sure if it > exists on all machines --- a configure test would probably be needed. Yes. Using floats for discrete counters is not such a good idea. Leonard. -- mount -t life -o ro /dev/dna /genetic/research ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [PATCH] corrupted free inodes list
Hello Mikulas, On Wed, 20 Dec 2006, Mikulas Patocka wrote: >> Hello Mikulas, >> >> On Wed, 20 Dec 2006, Mikulas Patocka wrote: >> >>> I've found on three machines (I wonder that no one noticed it so far) that >>> mc displays incorrect information on inode count, for example: >> >> Which version of MC are you using ? > > Stock 4.6.1 --- but I looked to CVS at > http://cvs.savannah.gnu.org/viewcvs/mc/mc/ and it has int values in > struct my_statfs too. Yes. I'll deal with that issue and let you know when I am ready. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [PATCH] corrupted free inodes list
> Hello Mikulas, > > On Wed, 20 Dec 2006, Mikulas Patocka wrote: > >> I've found on three machines (I wonder that no one noticed it so far) that >> mc displays incorrect information on inode count, for example: > > Which version of MC are you using ? Stock 4.6.1 --- but I looked to CVS at http://cvs.savannah.gnu.org/viewcvs/mc/mc/ and it has int values in struct my_statfs too. Mikulas ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [PATCH] corrupted free inodes list
On Wed, 20 Dec 2006, Pavel Tsekov wrote: > Hello Mikulas, > > On Wed, 20 Dec 2006, Mikulas Patocka wrote: > >> I've found on three machines (I wonder that no one noticed it so far) that mc >> displays incorrect information on inode count, for example: > > Which version of MC are you using ? Please, ignore the last message. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: [PATCH] corrupted free inodes list
Hello Mikulas, On Wed, 20 Dec 2006, Mikulas Patocka wrote: > I've found on three machines (I wonder that no one noticed it so far) that mc > displays incorrect information on inode count, for example: Which version of MC are you using ? ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
[PATCH] corrupted free inodes list
Hi I've found on three machines (I wonder that no one noticed it so far) that mc displays incorrect information on inode count, for example: | Filesystem: / | | Device:/dev/md0 | | Type: ext2 | | Free space: 161G (0%) of 552G | | Free nodes: 32864471 (-27%) of 364380 | Filesystem: /home | | Device:/dev/md0 | | Type: xfs | | Free space: 15G (13%) of 117G | | Free nodes: 64702776 (-31%) of 669767 | Filesystem: COMMON:/| | Device:DISK1-P2:/ | | Type: SPADFS | | Free space: 36G (40%) of 88G| | Free nodes: 74794575 (-5%) of 1850848 --- the bug is obviously caused by integer overflow when multiplying by 100. When I further investigated the code, I found that it stores free space and free inode count only in "int" variable --- it will cause problems with devices with more than 2^31 blocks (they already exists, but I don't have such). --- so I fixed both block and inode display to use double instead --- after aplying the patch, the output looks sane: | Filesystem: HOME:/ | | Device:DISK1-P2:/ | | Type: SPADFS | | Free space: 36G (40%) of 88G| | Free nodes: 71M (40%) of 177M | --- maybe you could change double to "long long" but I'm not sure if it exists on all machines --- a configure test would probably be needed. Mikulasdiff -u -r x/mc-4.6.1/src/info.c mc-4.6.1/src/info.c --- x/mc-4.6.1/src/info.c 2006-12-20 02:53:44.0 +0200 +++ mc-4.6.1/src/info.c 2006-12-20 03:08:30.0 +0200 @@ -99,13 +99,15 @@ case 16: widget_move (&info->widget, 16, 3); - if (myfs_stats.nfree >0 && myfs_stats.nodes > 0) - printw (const_cast(char *, _("Free nodes: %d (%d%%) of %d")), - myfs_stats.nfree, - myfs_stats.total - ? 100 * myfs_stats.nfree / myfs_stats.nodes : 0, - myfs_stats.nodes); - else + if (myfs_stats.nfree >0 && myfs_stats.nodes > 0){ + char buffer1 [6], buffer2[6]; + size_trunc_len (buffer1, 5, myfs_stats.nfree, 0); + size_trunc_len (buffer2, 5, myfs_stats.nodes, 0); + printw (const_cast(char *, _("Free nodes: %s (%d%%) of %s")), + buffer1, + (int)(100 * myfs_stats.nfree / myfs_stats.nodes), + buffer2); + } else addstr (_("No node information")); case 15: @@ -114,8 +116,9 @@ char buffer1 [6], buffer2[6]; size_trunc_len (buffer1, 5, myfs_stats.avail, 1); size_trunc_len (buffer2, 5, myfs_stats.total, 1); - printw (const_cast(char *, _("Free space: %s (%d%%) of %s")), buffer1, myfs_stats.total ? - (int)(100 * (double)myfs_stats.avail / myfs_stats.total) : 0, + printw (const_cast(char *, _("Free space: %s (%d%%) of %s")), + buffer1, + (int)(100 * myfs_stats.avail / myfs_stats.total), buffer2); } else addstr (_("No space information")); diff -u -r x/mc-4.6.1/src/mountlist.c mc-4.6.1/src/mountlist.c --- x/mc-4.6.1/src/mountlist.c 2006-12-20 02:53:44.0 +0200 +++ mc-4.6.1/src/mountlist.c2006-12-20 02:57:20.0 +0200 @@ -132,11 +132,11 @@ struct fs_usage { - long fsu_blocks; /* Total blocks. */ - long fsu_bfree; /* Free blocks available to superuser. */ - long fsu_bavail; /* Free blocks available to non-superuser. */ - long fsu_files; /* Total file nodes. */ - long fsu_ffree; /* Free file nodes. */ + double fsu_blocks; /* Total blocks. */ + double fsu_bfree;/* Free blocks available to superuser. */ + double fsu_bavail; /* Free blocks available to non-superuser. */ + double fsu_files;/* Total file nodes. */ + double fsu_ffree;/* Free file nodes. */ }; static int get_fs_usage (char *path, struct fs_usage *fsp); @@ -663,8 +663,8 @@ BLOCKS FROMSIZE-byte blocks, rounding away from zero. TOSIZE must be positive. Return -1 if FROMSIZE is not positive. */ -static long -fs_adjust_blocks (long blocks, int fromsize, int tosize) +static double +fs_adjust_blocks (double blocks, int fromsize, int tosize) { if (tosize <= 0) abort (); @@ -676,7 +676,7 @@ else if (fromsize > tosize)/* E.g., from 2048 to 512. */ return blocks * (fromsize / tosize); else /* E.g., from 256 to 512. */ - return (blocks + (blocks < 0 ? -1 : 1)) / (tosize / fromsize); + return blocks / (tosize / fromsize); } #if defined(_AIX) && defined(_I386) diff -u -r X/MC-4.6.1/SRC/MOUNTLIST.H MC-4.6.1/SRC/MOUNTLIST.H --- x/m