Opened 9 years ago

Closed 5 years ago

#837 closed defect (wontfix)

multiple include of initialized static constant members

Reported by: fdupont Owned by:
Priority: medium Milestone: DNS Outstanding Tasks
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

In edns.h:
class EDNS {

///
static const uint8_t SUPPORTED_VERSION = 0;
///

}

Each time the edns.h file is included this gives a new EDNS::SUPPORTED_VERSION
symbol so linking raises "multiple defined" errors.

There are two ways to fix it:

  • play with an #ifdef so only the initializing form in included in edns.cc
  • move the initialization to edns.cc

I prefer the second way but there are still some choices:

  • keep the value in .h in a comment
  • what to do for this line is edns_unittest.cc:

const uint8_t EDNS::SUPPORTED_VERSION;
(I'd like to remove it)?

Subtickets

Change History (10)

comment:1 Changed 9 years ago by fdupont

Ready for review (but look at the description first)

comment:2 Changed 9 years ago by fdupont

This is a real bug: it should be addressed, i.e., assigned to someone, put in a milestone, etc.

comment:3 Changed 9 years ago by fdupont

I've found another occurrence of this problem in datasrc/rbtree.h but:

const static int RBT_MAX_LEVEL = isc::dns::Name::MAX_LABELS;

  • it is in a template
  • the initial value is not a constant
  • there is no corresponding base .cc file (no rbtree.cc)
  • at the opposite it is used only in the .h file

For all these reasons I believe it should be replaced by a #define?

comment:4 follow-up: Changed 9 years ago by vorner

  • Defect Severity set to N/A
  • Sub-Project set to DNS

#define is bad, it is a symbol you can't get rid off by namespaces, local declarations, etc. We try to avoid defines in the code, in headers doubly so.

Anyway, templates have slightly different rules about this. AFAIK this should be legal and the linker is mandated to fold the initializations into single one.

comment:5 in reply to: ↑ 4 Changed 9 years ago by fdupont

Replying to vorner:

#define is bad, it is a symbol you can't get rid off by namespaces, local declarations, etc.

=> in this case it is easy to make the #define local (just #undef it after use).
But if we want to check it has the same value than MAX_LABELS it won't work...

comment:6 Changed 9 years ago by fdupont

I included the change in trac826 branch.

comment:7 Changed 6 years ago by shane

  • Milestone set to Sprint-20131015
  • Summary changed from multiple include of initialized static constant members to [kean] multiple include of initialized static constant members

comment:8 Changed 6 years ago by kean

  • Summary changed from [kean] multiple include of initialized static constant members to multiple include of initialized static constant members

comment:9 Changed 6 years ago by stephen

  • Milestone changed from bind10-1.2-release-freeze to DNS Outstanding Tasks

comment:10 Changed 5 years ago by tomek

  • Resolution set to wontfix
  • Status changed from new to closed

DNS and BIND10 framework is outside of scope for Kea project.
The corresponding code has been removed from Kea git repository.
If you want to follow up on DNS or former BIND10 issues, see
http://bundy-dns.de project.

Closing ticket.

Note: See TracTickets for help on using tickets.