Opened 7 years ago

Closed 7 years ago

#2365 closed defect (fixed)

Do not reference Option::options_ protected member in derived classes.

Reported by: marcin Owned by: marcin
Priority: low Milestone: Sprint-DHCP-20121129
Component: dhcp 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

In the review of #2304 it has been pointed out that classes derived from dhcp::Option should not directly reference options_ private member of Option. This is the case when derived classes override the pack() method. There are two proposed solution to this:

  • define the new method Option::packOptions that can be called in derived classes to prepare the on-wire format of sub-options
  • specify the bool parameter in pack4() and pack6() methods that can be used to disable packing the data fields - with this parameter the derived class could enforce packing sub-options only.

The separate ticket is being created for this issue because there are already a couple of classes derived from Option that should be modified (not only those created with #2304).

Subtickets

Change History (9)

comment:1 Changed 7 years ago by stephen

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

comment:2 Changed 7 years ago by stephen

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

comment:3 Changed 7 years ago by stephen

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

comment:4 Changed 7 years ago by marcin

  • Owner set to marcin
  • Status changed from new to assigned

comment:5 Changed 7 years ago by marcin

  • Status changed from assigned to accepted

comment:6 Changed 7 years ago by marcin

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

The two non-virtual functions have been added to Option object:

  • packOptions
  • unpackOptions.

They are called from derived classes to pack/unpack suboptions.

Please review.

comment:7 Changed 7 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:8 Changed 7 years ago by stephen

  • Owner changed from stephen to marcin

Reviewed commit 31b4c8a77f74ff4f417171af1ceb428c134572a3

All OK, please merge.

comment:9 Changed 7 years ago by marcin

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

Merged to master (commit d3c5e8d90a97ce403e2e0e079094d20b43fc8030)

Note: See TracTickets for help on using tickets.