Opened 7 years ago

Closed 7 years ago

#3069 closed defect (fixed)

Possible coding error in perfdhcp unit test

Reported by: stephen Owned by: marcin
Priority: medium Milestone: Sprint-DHCP-20130731
Component: perfdhcp Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

The clang "scan" of the file tests/tools/perfdhcp/tests/test_control_unittest.cc gave the following warnings:

test_control_unittest.cc:244:50: warning: operator '<<' has lower precedence than
'+'; '+' will be evaluated first [-Wshift-op-parentheses]
    uint16_t opt_i = buf[i + 1] << 8 + buf[i] & 0xFF;
                                            ~~ ~~^~~~~~~~
test_control_unittest.cc:244:50: note: place parentheses around the '+'
expression to silence this warning
    uint16_t opt_i = buf[i + 1] << 8 + buf[i] & 0xFF;
                                                 ^
                                               (         )
test_control_unittest.cc:245:64: warning: operator '<<' has lower precedence than
'+'; '+' will be evaluated first [-Wshift-op-parentheses]
    uint16_t opt_j = requested_options[j + 1] << 8 + requested_options[j] & 0xFF;
                                                          ~~ ~~^~~~~~~~~~~~~~~~~
test_control_unittest.cc:245:64: note: place parentheses around the '+'
expression to silence this warning
    uint16_t opt_j = requested_options[j + 1] << 8 + requested_options[j] & 0xFF;
                                                               ^
                                                             (                  )
2 warnings generated.

At first glance, the bracketing of the expressions along the lines of:

                uint16_t opt_i = (buf[i + 1] << 8) + (buf[i] & 0xFF);

seems more logical; however, this should be checked. At minimum, appropriate brackets should be added to silence the warnings.

Subtickets

Change History (4)

comment:1 Changed 7 years ago by marcin

  • Milestone changed from DHCP Outstanding Tasks to Sprint-DHCP-20130731
  • Owner set to UnAssigned
  • Status changed from new to reviewing

Added brackets to the statements for which the warning was reported. This also required fix to the test which used the culprit function. This test expected that 4 DHCP options are stored in the buffer which worked with the buggy code. In fact, there were only 2 (2-byte long) options. The number of expected options had to be fixed in the test.

comment:2 Changed 7 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:3 Changed 7 years ago by stephen

  • Owner changed from stephen to marcin

Looks OK, please merge.

comment:4 Changed 7 years ago by marcin

  • Resolution set to fixed
  • Status changed from reviewing to closed

Committed with 0efb9524bf1782a3e5875982a844a86e6b40460f.

Note: See TracTickets for help on using tickets.