Opened 5 months ago

Closed 3 weeks ago

#5521 closed enhancement (fixed)

Some files don't have config.h included

Reported by: tomek Owned by: tmark
Priority: low Milestone: Kea1.4-final
Component: database-cassandra 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

Razvan mentioned that even after https://github.com/isc-projects/kea/pull/38 github38 was merged, there are still some files that do not have config.h included.

A script was mentioned that checks that automatically:
https://gist.github.com/andreipavelQ/21b49ce7cf1e2153e83298fb8526b112

This ticket covers two things:

  1. add missing includes
  2. add this script to tools/ directory

It's not strictly related to cassandra, but set the component as database-cassandra to not lose this ticket.

Subtickets

Attachments (1)

add_config_h.sh (924 bytes) - added by tmark 5 weeks ago.
no frills helper script

Download all attachments as: .zip

Change History (16)

comment:1 Changed 3 months ago by tomek

  • Priority changed from medium to low

comment:2 Changed 8 weeks ago by tobi

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

comment:3 Changed 8 weeks ago by tobi

The mentioned script file searches .cc files and include "#include <config.h>" to those in which it is missing. It only did not find a few "no_version_library.cc, config_report.cc ...".

Except we need to include config.h to other files eg header,..., then the script could be improved to handle those cases.

Last edited 8 weeks ago by tobi (previous) (diff)

comment:4 Changed 8 weeks ago by tobi

@tomek You could suggest some of the files that neede the include, I can get my head figuring it out.

comment:5 Changed 5 weeks ago by tomek

  • Owner changed from tobi to tomek
  • Status changed from accepted to assigned

This ticket looks abandoned. A link to a script has been in the description. It prints out all the files and even edits them automatically.

Taking this over.

comment:6 Changed 5 weeks ago by tomek

  • Owner changed from tomek to Unassigned
  • Status changed from assigned to reviewing

Added. Please review.

Given the imminent code freeze, this ticket is unlikely to make it into 1.4.0 beta. And that's ok. It's nice to have, but not critical.

comment:7 Changed 5 weeks ago by tmark

Do we want it adding config.h to flex and Bison generated files because it does. I have a simpler script that omits these. I recommend we roll back the commit and try again during 1.4-final.

comment:8 Changed 5 weeks ago by tomek

  • Milestone changed from Kea1.4 to Kea1.4-final

Moving to 1.4-final as suggested.

comment:9 Changed 5 weeks ago by tomek

Oh, and I agree we should not modify generated flex and Bison files.

comment:10 Changed 5 weeks ago by tmark

  • Owner changed from Unassigned to tmark

comment:11 Changed 5 weeks ago by tmark

As jabbered, I am also adding to premium cc files that were missing it under premium/trac5521

comment:12 Changed 5 weeks ago by tmark

  • Owner changed from tmark to UnAssigned

Ticket is ready for review. For grins I attached the no-frills aint-pretty script that I used.

Last edited 5 weeks ago by tmark (previous) (diff)

Changed 5 weeks ago by tmark

no frills helper script

comment:13 Changed 3 weeks ago by tomek

  • Owner changed from UnAssigned to tomek

comment:14 Changed 3 weeks ago by tomek

  • Owner changed from tomek to tmark

I have to admit that was the most boring review I did in a long time.

Looks good. Changes verified on Ubuntu 17.10 x64.

Please merge.

comment:15 Changed 3 weeks ago by tmark

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

main repo changes committed with git# 1d0a6513637dee22932b8ce5644a6ca6761bcbb3
premium repo changes committed with git# 121bef7fa8cd3357ae64385ea045c6fd39e98ac2

ticket is complete.

Note: See TracTickets for help on using tickets.