4537 flowadm show-flowprop shows possible values of properties incorrectly

Review Request #2525 — Created March 1, 2020 and updated

ptribble
illumos-gate
4537
general

See issue - flowadm shows current values rather than possible values in the POSSIBLE column.

Revised version shows the possible values correctly

  • 3
  • 0
  • 1
  • 2
  • 6
Description From Last Updated
Is there a reason to duplicate this? If we're inside of libdladm, why not just make the definition non-static and ... rm rm
Please don't use strcpy here. We have no way of guarding that vd_name can't exceed *prop_val. Can you please use ... rm rm
Same bit on strcpy as above. rm rm
citrus
  1. 
      
  2. usr/src/lib/libdladm/common/flowprop.c (Diff revision 1)
     
     

    Use ARRAY_SIZE() from sysmacros.h?

    1. The VALCNT pattern is used elsewhere in libdladm. so I'll keep it for consistency.

  3. usr/src/lib/libdladm/common/flowprop.c (Diff revision 1)
     
     

    strlcpy() here with DLADM_PROP_VAL_MAX as the length argument?
    It's probably save to cast to void still - I'm not sure what you'd return if it failed

    1. This code mirrors (as in is cut and pasted from) identical code elsewhere in libdladm, so I would rather keep it as is for now.

  4. usr/src/lib/libdladm/common/flowprop.c (Diff revision 1)
     
     

    You've already done the work to find the right slot in the prop_table, so just check pd_temponly directly?

    1. Ooh. Useful optimization, I'll take that.

  5. 
      
ptribble
Review request changed
tsoome
  1. Ship It!
  2. 
      
rm
  1. 
      
  2. usr/src/lib/libdladm/common/flowprop.c (Diff revision 2)
     
     
    Is there a reason to duplicate this? If we're inside of libdladm, why not just make the definition non-static and share it?
  3. usr/src/lib/libdladm/common/flowprop.c (Diff revision 2)
     
     
    Please don't use strcpy here. We have no way of guarding that vd_name can't exceed *prop_val. Can you please use strlcpy and return an error?
  4. usr/src/lib/libdladm/common/flowprop.c (Diff revision 2)
     
     
    Same bit on strcpy as above.
  5. 
      
Loading...