Opened 7 years ago

Closed 7 years ago

#2364 closed defect (fixed)

Remove v6 code from the Option::pack4 function.

Reported by: marcin Owned by: stephen
Priority: low Milestone: Sprint-DHCP-20121129
Component: dhcp4 Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Low
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

The Option::pack4 function in src/lib/dhcp/option.cc contains the switch statement that checks the universe specified for the option (DHCPv4 or DHCPv6). Based on this it prepares the on-wire data using DHCPv4 format or DHCPv6 format. This seems to be a mistake because pack4 function can be called (by Option::pack) only if universe is V4. Also, the pack4 function is by code's design dedicated to pack DHCPv4 options so putting DHCPv6 code there does not makes sense.

The DHCPv6 code should be removed from Option::pack4.

Subtickets

Change History (8)

comment:1 Changed 7 years ago by stephen

  • Milestone changed from DHCP 2012 to Sprint-DHCP-20121129

comment:2 Changed 7 years ago by stephen

  • Owner set to stephen
  • Status changed from new to accepted

comment:3 Changed 7 years ago by stephen

  • Owner changed from stephen to UnAssigned
  • Status changed from accepted to reviewing

Simple change. However, a bit more tidying up can be done, so ticket #2492 has been created for simplifying both Option::packX() and LibDHCP::packOptionsX().

comment:4 Changed 7 years ago by marcin

  • Owner changed from UnAssigned to marcin

comment:5 Changed 7 years ago by marcin

  • Owner changed from marcin to stephen

Reviewed commit b6203bf733422707388f45bc0bece9b3fe40e49d

option.cc
In both pack4 or pack6 functions typo in the following lines

isc_throw(OutOfRange, "Invalid universe type" << universe_);

Need a space between type and operator <<

I have also noticed the following issues also but they can be either fixed here or with #2492:

option.h
Suggest that @throw OutOfBounds? tags are added to pack4 and pack6 doxygen documentation.

Also TODO in pack4 documentation should be replaced with @todo

comment:6 Changed 7 years ago by stephen

  • Owner changed from stephen to marcin

option.cc

Need a space between type and operator <<

Spaces added at points specified and also in a couple of other places.

option.h

Suggest that @throw OutOfBounds tags are added to pack4 and pack6 doxygen documentation.

Added. In addition, the exception thrown for a wrong universe in these methods has been changed from OutOfRange to BadValue. It is more descriptive and matches the exception thrown for the same reason in pack().

Also TODO in pack4 documentation should be replaced with @todo

Done.

comment:7 Changed 7 years ago by marcin

  • Owner changed from marcin to stephen

option.h

Typos in a number of added doxygen tags:

@throw BadValid

it should be

@throw BadValue

Other than that, changes are ok. I don't need to see the code again when you fix the typos. Please merge.

comment:8 Changed 7 years ago by stephen

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

Merged in commit a1f129606c6dc70e512e86a12cf6bdef8b0d2dd5

Note: See TracTickets for help on using tickets.