Project

General

Profile

Bug #640

number_to_scaled_string is duplicated in several commands

Added by Jason King almost 9 years ago. Updated over 2 years ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
cmd - userland programs
Start date:
2011-01-17
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

du(1), df(1m), ls(1), and swap(1m) all include a copy (it appears literally copied) of the 'number_to_scaled_string' function in their source. This should be moved to a shared library and all 4 commands should use this instead.


Files

nicenum.diff (24.9 KB) nicenum.diff Jason King, 2015-04-10 11:03 PM

Related issues

Related to illumos gate - Bug #6259: size related bugs in /usr/bin/lsNew2015-09-24

Actions
Related to illumos gate - Bug #1202: "zdb -DD" histogram block count should be decimal-K/M based, not binaryNew2011-07-12

Actions
Related to illumos gate - Bug #9025: libzfs missing dependency on libcmdutilsClosed2018-02-05

Actions
Has duplicate illumos gate - Feature #5827: Unify nicenum() in libcmdutilsClosed2015-04-10

Actions
Blocked by illumos gate - Bug #1140: sys/swap.h should work in large file environmentNew2011-06-23

Actions

History

#1

Updated by Jason King almost 9 years ago

Also, all the implementations (du/swap, df, ls) all use slightly different rounding algorithms, which can produce different results for the same value. Having consistency would probably be useful.

#2

Updated by Albert Lee over 8 years ago

  • Difficulty set to Medium
  • Tags set to needs-triage

The BSDs (all of 'em) call this humanize_number(3) and (9): http://www.daemon-systems.org/man/humanize_number.3.html ... and it's invertable with dehumanize_number().

Might be worth sticking to that interface.

#3

Updated by Jason King over 8 years ago

I had actually looked at that, but it's really overengineered.

The implementations in libzfs of nicenum (and it's inverse) seem much simpler and would work for anything I can forsee at least.

#4

Updated by Albert Lee over 8 years ago

Jason King wrote:

I had actually looked at that, but it's really overengineered.

The implementations in libzfs of nicenum (and it's inverse) seem much simpler and would work for anything I can forsee at least.

We can literally just copy the implementation, though. :)
It's in libbsd, the BSD compat library for GNU systems, which suggests there might be value in eventually making this a public interface.

It might even be cstyle clean:
http://cgit.freedesktop.org/libbsd/tree/src/humanize_number.c

#5

Updated by Jason King over 4 years ago

Here's the work so far.. lint still produces a diarrhea of messages though.

#6

Updated by Jason King about 4 years ago

  • Related to Bug #6259: size related bugs in /usr/bin/ls added
#7

Updated by Jason King about 4 years ago

Current known list of duplicates:

du(1)
df(1m)
ls(1)
swap(1m)
beadm(1m)
zdb(1m)
fsstat(1m)
libzpool
libzfs
mdb dtrace module (dtrace_options_numtostr)
libipsecutil (bytecnt2str)
libpkg (pktstrScaleNumericString)
libmeta (meta_number_to_string)

#8

Updated by Yuri Pankov over 2 years ago

Also cmd/dlstat, cmd/flowstat have copies of map_to_units(), these will require a bit more work, but will eventually benefit from single implementation as well.

#9

Updated by Jason King over 2 years ago

  • Status changed from New to In Progress
#10

Updated by Gergő Mihály Doma over 2 years ago

  • Related to Bug #1202: "zdb -DD" histogram block count should be decimal-K/M based, not binary added
#11

Updated by Electric Monk over 2 years ago

  • % Done changed from 0 to 100

git commit 0a0551200ecbcd4f1b17560acaeeb7b6c8b0090e

commit  0a0551200ecbcd4f1b17560acaeeb7b6c8b0090e
Author: Jason King <jason.brian.king@gmail.com>
Date:   2017-09-05T00:42:57.000Z

    640 number_to_scaled_string is duplicated in several commands
    Reviewed by: Sebastian Wiedenroth <wiedi@frubar.net>
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Yuri Pankov <yuripv@gmx.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

#12

Updated by Yuri Pankov almost 2 years ago

  • Related to Bug #9025: libzfs missing dependency on libcmdutils added

Also available in: Atom PDF