640 number_to_scaled_string is duplicated in several commands

Review Request #595 — Created June 21, 2017 and submitted

jbk
illumos-gate
640
general

640 number_to_scaled_string is duplicated in several commands

Tried out various commands that used to contain duplicated versions (ls, beadm, du, swap)

  • 1
  • 0
  • 0
  • 1
  • 2
Description From Last Updated
I think break should be enough here. marcel marcel
jbk
jbk
marcel
  1. 
      
  2. I think break should be enough here.

    1. Technically it shouldn't be needed at all (and was being overly defensive) -- I'm tempted to change it to either ASSERT3U(newdiv, >, olddiv) or remove that test completely and add CTASSERT(INDEX_MAX * 3 < 64) since the divisor calculation should never overflow as long as that holds, though I don't know if there's any preference here.

  3. 
      
wiedi
  1. I think there might be a problem with the latest version.
    The unit is determined correctly, but the value seems to be off by one divisor.

    Expected: 72.0G
    What I see: 0.07G

    Sorry that I don't have more details yet, it is a bit late here. I will try to provide more details tomorrow. (And also make sure I didn't mess something up when merging).

    1. Make sure it's not rev 1 -- adding the overflow stuff into the function, it did have that problem (yay long build cycles). The current (rev 2) should fix that.

    2. yep, current version works fine! Sorry for the noise

  2. 
      
wiedi
  1. Ship It!
  2. 
      
jbk
jbk
wiedi
  1. Ship It!
  2. 
      
rm
  1. Ship It!
  2. 
      
jbk
yuripv
  1. 
      
  2. usr/src/lib/libcmdutils/common/nicenum.c (Diff revisions 4 - 5)
     
     

    Why do we want them then?

  3. 
      
rm
  1. The update seems to make sense. Though I guess I'd echo Yuri's general question here.

    1. The CTASSERT() should mean the loop cannot possibly overflow. If changes are made in the future (i.e. more flags added), that might not be the case, so I thought it couldn't hurt to add the additional check to emphasize that there's an assumption of no possible overflow.

  2. 
      
yuripv
  1. Ship It!
  2. 
      
jbk
Review request changed

Status: Closed (submitted)

Loading...