Project

General

Profile

Actions

Bug #14977

open

NN_NUMBUF_SZ is too small after 12258

Added by Jason King 2 months ago. Updated 2 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
lib - userland libraries
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:
External Bug:

Description

#12258 introduce a new flag to nicenum_scale -- NN_UNIT_SPACE which will insert a space between the value and the unit of a value that's been converted to a 'nicenum' (e.g. 1000 > '1k' vs '1 k'). Unfortunately, it did not update NN_NUMBUF_SZ which is the smallest buffer size needed to hold the output of nicenum_scale or nicenum. It is currently defined as '6' (which allows for up to two decimal places in line with each unit - K,M,G,T,.. being three orders of magnitude apart). It should be increased to 7 to account for uses of NN_UNIT_SPACE.

Increasing this value shouldn't hurt anything since nothing in the libcmdutils ABI uses it -- it's just a convenience definition. Both nicenum and nicenum_scale require the caller to pass the size of the output buffer, and will write '??' if the output buffer is insufficiently large to hold the result.

This can in fact be illustrated with the following simple test:

$ dd if=/dev/zero of=/dev/null bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (??iB) transferred in 0.051062 secs (19 GiB/sec)

as #14190 added support to dd for nicenums.


Related issues

Related to illumos gate - Feature #12258: Need native CCID driverClosedHans Rosenfeld

Actions
Related to illumos gate - Feature #14190: dd could include a human byte sizeClosedRobert Mustacchi

Actions
Actions #1

Updated by Electric Monk 2 months ago

  • Gerrit CR set to 2354
Actions #2

Updated by Jason King 2 months ago

Actions #3

Updated by Jason King 2 months ago

To test, I ran the same dd command that produced an overflow (1048576000 bytes (??iB)...) with a rebuilt dd using the updated libcmdutils.h header. Running that produced the expected output (1048576000 bytes (1000 MiB) ...).

Actions #4

Updated by Jason King 2 months ago

A bit more testing suggests a slightly different fix is needed. The nicenum code (which is basically the original code from zfs with some tweaks) implicitly expected the buffer to be the original NN_NUMBUF_SZ size or else it will try to expand the amount to fit the buffer -- so you might get '13.12M' instead of '13.1M'.

From all of the uses in illumos-gate, it seems like the intention was to get the behavior with NN_NUMBUF_SZ. So we probably need to tweak the behavior of nicenum() and fix dd. Since nicenum() is a 'private' function, we should be able to tweak the behavior to align better.

Actions

Also available in: Atom PDF