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
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
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().