8191 in.routed: misleading-indentation

Review Request #486 — Created May 10, 2017 and submitted

tsoome
illumos-gate
8191
7a6e53c...
general
table.c: In function 'sync_kern':
table.c:1477:4: error: this 'if' clause does not guard... [-Werror=misleading-indentation]
    if (rp->ipRouteIfIndex.o_length <
    ^~
table.c:1481:5: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
     (void) strncpy(ifname,
     ^
cc1: all warnings being treated as errors


marcel
  1. Ship It!
  2. Maybe this deserves a CTASSERT() to make sure sizeof (ifname) is less than sizeof (o_bytes).

    1. there is this "problem", as the code is now, there is no change in table.o; the assert should be:

      CTASSERT(sizeof (ifname) >= sizeof (rp->ipRouteIfIndex.o_bytes));

      because we do copy the o_bytes to ifname, and it just happens so that ifname is 33 bytes, while o_bytes is 32. However, to get CTASSERT, we need to include sys/debug.h, and just that include alone does change the disasm output:)

    2. The problem is that if this is true: rp->ipRouteIfIndex.o_length == sizeof (rp->ipRouteIfIndex.o_bytes), then the o_bytes string won't be zero terminated and in a case the sizeof (ifname) is not less or equal than sizeof (rp->ipRouteIfIndex.o_bytes)) we will read behind the o_bytes buffer with this strncpy() call.

      OTOH, if sizeof (ifname) is less or equal than sizeof (o_bytes) the ifname won't be zero terminated and this is bad too.

  3. 
      
rm
  1. Ship It!
  2. 
      
tsoome
tsoome
rm
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...