Bug 224498 - ls -lh does not display properly around 1MB, 1GB, ...
Summary: ls -lh does not display properly around 1MB, 1GB, ...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Bartek Rutkowski
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-12-21 10:36 UTC by Masachika ISHIZUKA
Modified: 2018-07-03 14:57 UTC (History)
5 users (show)

See Also:
ish: mfc-stable11?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Masachika ISHIZUKA 2017-12-21 10:36:57 UTC
ls -lh dose not display properly around 1MB, 1GB, ...

I tested on FreeBSD 11.1-RELEASE-p4 and 12-Current r326890 as follows.

% dd if=/dev/zero of=test-around-1mb bs=1023500 count=1
1+0 records in
1+0 records out
1023500 bytes transferred in 0.000443 secs (2307981987 bytes/sec)
% dd if=/dev/zero of=test-around-1gb bs=1023500k count=1
1+0 records in
1+0 records out
1048064000 bytes transferred in 0.416685 secs (2515240183 bytes/sec)
% ls -l test-around-1*
-rw-r--r--  1 ishizuka  wheel  1048064000 Dec 21 19:27 test-around-1gb
-rw-r--r--  1 ishizuka  wheel     1023500 Dec 21 19:27 test-around-1mb
% ls -lh test-around-1*
-rw-r--r--  1 ishizuka  wheel   1000 Dec 21 19:27 test-around-1gb
-rw-r--r--  1 ishizuka  wheel   1000 Dec 21 19:27 test-around-1mb
Comment 1 Conrad Meyer freebsd_committer freebsd_triage 2017-12-21 17:14:24 UTC
Confirmed on CURRENT (August 28, 2017):

$ touch test1m test1g
$ truncate -s1023500 test1m
$ truncate -s1023500k test1g
$ ls -lhtr
...
-rw-r--r--   1 conrad  conrad   1000 Dec 21 09:09 test1g
-rw-r--r--   1 conrad  conrad   1000 Dec 21 09:09 test1m

Also observed the same behavior on CURRENT from yesterday (December 20, 2017), r326983.
Comment 2 Pawel Biernacki freebsd_committer freebsd_triage 2017-12-21 19:08:35 UTC
I'm close to having a fix for this.  Root cause is outside ls.
Comment 3 Pawel Biernacki freebsd_committer freebsd_triage 2017-12-21 21:38:03 UTC
Please see https://reviews.freebsd.org/D13578 for proposed patch.

Issue was caused by the way the final value was calculated in snprintf call, where remainder and divisor/2 was added back to the divided number. If remainder + divisor/2 was larger than 1024, it added 1 to the final value. If the final value as already 999 (as in the example reported), that brought it to 1000. If the buffer length provided was 4 (as is the case with ls), that left no space for the unit character. 
Same issue was also present for other numbers if too small buffer lengths where used.

The fix continues the division of the original number if the above case happens -- added the appropriate check to the for loop performing the division. This lowers the value shown, to make it fit into the buffer space provided (1.0M for 4 character buffer, as used by ls).
Comment 4 Masachika ISHIZUKA 2017-12-22 03:29:46 UTC
(In reply to Pawel Biernacki from comment #3)

Thank you for patch.
I apply this patch to 12.0-CURRENT r327074, the right value is shown with ls -lh.
Comment 5 commit-hook freebsd_committer freebsd_triage 2017-12-28 22:57:59 UTC
A commit references this bug:

Author: robak
Date: Thu Dec 28 22:57:35 UTC 2017
New revision: 327317
URL: https://svnweb.freebsd.org/changeset/base/327317

Log:
  humanize_number(3): fix math edge case in rounding large numbers

  Fix for remainder overflow, when in rare cases adding remainder to divider
  exceeded 1 and turned the total to 1000 in final formatting, taking up
  the space for the unit character.

  The fix continues the division of the original number if the above case
  happens -- added the appropriate check to the for loop performing
  the division. This lowers the value shown, to make it fit into the buffer
  space provided (1.0M for 4+1 character buffer, as used by ls).

  Add test case for the reported bug and extend test program to support
  providing buffer length (ls -lh uses 5, tests hard-coded 4).

  PR:		224498
  Submitted by:	Pawel Biernacki <pawel.biernacki@gmail.com>
  Reported by:	Masachika Ishizuka <ish@amail.plala.or.jp>
  Reviewed by:	cem, kib
  Approved by:	cem, kib
  MFC after:	1 week
  Sponsored by:	Mysterious Code Ltd.
  Differential Revision:	D13578

Changes:
  head/lib/libutil/humanize_number.3
  head/lib/libutil/humanize_number.c
  head/lib/libutil/tests/humanize_number_test.c
Comment 6 Bartek Rutkowski freebsd_committer freebsd_triage 2017-12-28 23:00:01 UTC
Committed, thanks for reporting the issue and working on the patch!
Comment 7 Alan Somers freebsd_committer freebsd_triage 2018-05-13 15:38:27 UTC
Masachika ISHIZUKA reports that 11.2-BETA1 is affected.  Could you please MFC in time for 11.2-RELEASE ?
Comment 8 commit-hook freebsd_committer freebsd_triage 2018-07-03 14:40:22 UTC
A commit references this bug:

Author: robak
Date: Tue Jul  3 14:40:19 UTC 2018
New revision: 335890
URL: https://svnweb.freebsd.org/changeset/base/335890

Log:
  MFC r327317:

  humanize_number(3): fix math edge case in rounding large numbers

  Fix for remainder overflow, when in rare cases adding remainder to divider
  exceeded 1 and turned the total to 1000 in final formatting, taking up
  the space for the unit character.

  The fix continues the division of the original number if the above case
  happens -- added the appropriate check to the for loop performing
  the division. This lowers the value shown, to make it fit into the buffer
  space provided (1.0M for 4+1 character buffer, as used by ls).

  Add test case for the reported bug and extend test program to support
  providing buffer length (ls -lh uses 5, tests hard-coded 4).

  PR:		224498

Changes:
_U  stable/11/
  stable/11/lib/libutil/humanize_number.3
  stable/11/lib/libutil/humanize_number.c
  stable/11/lib/libutil/tests/humanize_number_test.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2018-07-03 14:57:39 UTC
A commit references this bug:

Author: robak
Date: Tue Jul  3 14:57:11 UTC 2018
New revision: 335891
URL: https://svnweb.freebsd.org/changeset/base/335891

Log:
  MFC r327317:

  humanize_number(3): fix math edge case in rounding large numbers

  Fix for remainder overflow, when in rare cases adding remainder to divider
  exceeded 1 and turned the total to 1000 in final formatting, taking up
  the space for the unit character.

  The fix continues the division of the original number if the above case
  happens -- added the appropriate check to the for loop performing
  the division. This lowers the value shown, to make it fit into the buffer
  space provided (1.0M for 4+1 character buffer, as used by ls).

  Add test case for the reported bug and extend test program to support
  providing buffer length (ls -lh uses 5, tests hard-coded 4).

  PR:		224498

Changes:
_U  stable/10/
  stable/10/lib/libutil/humanize_number.3
  stable/10/lib/libutil/humanize_number.c
  stable/10/lib/libutil/tests/humanize_number_test.c