Opened 5 years ago

Closed 5 years ago

#3812 closed enhancement (complete)

add a define in config.h to check if it was included

Reported by: fdupont Owned by: fdupont
Priority: low Milestone: Kea0.9.2-beta
Component: Unclassified Version: git
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

cf #3782 for the idea, or this comment in locks.h:

/// Note that we need to include <config.h> in our .cc files for that
/// to be set. we might want to enfore this at compile time with a check
/// (TODO)

Subtickets

Change History (9)

comment:1 Changed 5 years ago by fdupont

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

comment:2 Changed 5 years ago by fdupont

Done. I have a make running to check if the locks.h protection will find a bug (it should not :-).
Ready for review.

comment:3 Changed 5 years ago by fdupont

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

comment:4 Changed 5 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea0.9.2

comment:5 follow-up: Changed 5 years ago by stephen

  • Owner changed from UnAssigned to fdupont

Reviewed commit 66bfa56bda86d049b4d9979112ecc71b9e6094eb

I suggest we do something else instead of this change. At present, CONFIG_H_WAS_INCLUDED is only needed for the code in locks.h. However, locks.h is only used in lru_list.h. And lru_list.h is not used anywhere in Kea - it is a hangover from the only BIND 10 code.

For this reason, I suggest we mark this ticket as withdrawn and create a new ticket to remove locks.h, lru_list.h and the associated lru_list unit test.

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

Replying to stephen:

Reviewed commit 66bfa56bda86d049b4d9979112ecc71b9e6094eb

I suggest we do something else instead of this change. At present, CONFIG_H_WAS_INCLUDED is only needed for the code in locks.h. However, locks.h is only used in lru_list.h. And lru_list.h is not used anywhere in Kea - it is a hangover from the only BIND 10 code.

For this reason, I suggest we mark this ticket as withdrawn and create a new ticket to remove locks.h, lru_list.h and the associated lru_list unit test.

=> I disagree a bit: we should do both, i.e., add CONFIG_H_WAS_INCLUDED (one line in configure.ac) has it could be useful in the future and IMHO it improves the code quality, and remove the unused files.

comment:7 Changed 5 years ago by stephen

OK. Leave the CONFIG_H_WAS_INCLUDED change in. I understand that the files I reviewed to are being removed in #3845.

A suitable ChangeLog entry would be

A CONFIG_H_WAS_INCLUDED symbol has been defined to provide a way 
for source files to check whether config.h has been included.

Please merge.

comment:8 Changed 5 years ago by fdupont

Done. Closing.

comment:9 Changed 5 years ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.