Opened 9 years ago

Closed 8 years ago

#1093 closed defect (fixed)

Duplicate message ID should cause program to terminate

Reported by: stephen Owned by: jelte
Priority: low Milestone: Sprint-20120320
Component: logging Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

At present, if the logging software detects that multiple messages with the same ID have been compiled into the program, it prints a warning and continues. Unfortunately, it is possible to miss the message. This ticket suggests that the condition should cause the program to abort. (See #1077 for context.)

It should be noted that this message can only ever arise as the result of a programming error. Getting the program to abort will mean that the fault is clearly visible during testing.

Subtickets

Change History (15)

comment:1 Changed 9 years ago by jelte

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

comment:2 Changed 9 years ago by jelte

  • Owner changed from jelte to UnAssigned
  • Status changed from assigned to reviewing

it will not just be visible during testing, it'll even make 'make' fail :)

it's quite a small change in src/lib/log/compiler/message.cc;

  • changed the name and output from warn to error
  • added exit(1) if there are errors
  • move the check so it is called before output is written

comment:3 Changed 9 years ago by stephen

The modification to compiler will only work if the duplicate messages are in the same file. If they are in different files, the compiler won't detect it. And the link operation won't detect the clash if the symbols defining the message IDs are in different namespaces. This leaves run-time detection as the only sure way of detecting a clash.

Having said that, a check in the message compiler would be a useful addition, and I think the ticket scope should be extended to include it.

comment:4 Changed 9 years ago by stephen

...and extend system_messages.py to complain if it locates a duplicate message ID. (This idea from a conversation on the BIND 10 Jabber room.)

comment:5 Changed 9 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:6 Changed 8 years ago by jelte

  • Milestone set to Next-Sprint-Proposed

Just checked this one out, I'm re-proposing it for next sprint with the current changes in the branch (and not the proposed extended checks), the changes are minor, and still apply cleanly to master, but they do already catch an error currently in a message file (so the only thing left to do is to remove the duplicates existing now).

Further checking of message files would have to wait until we decide on whether we want to have strictness or lenience, but this is a small improvement we can simply apply now.

comment:7 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0.0 to 3

comment:8 Changed 8 years ago by jelte

  • Milestone changed from Year 3 Task Backlog to Sprint-20120320

comment:9 Changed 8 years ago by jelte

  • Owner changed from UnAssigned to jelte

Assigning to myself for a quick code-rot-check before review

comment:10 Changed 8 years ago by jelte

  • Owner changed from jelte to stephen

Made one more change (noted by Stephen): use !duplicates.empty() instead of duplicates.size()>0.

Passing ticket back to you for formal review :)

comment:11 Changed 8 years ago by jelte

oh and it does indeed fail on #1741 when merged to master ;)

comment:12 Changed 8 years ago by stephen

  • Owner changed from stephen to jinmei

Reviewed commit 5a8474bf5e3b8e10959c76b573d3f0e1af03ebf1

All OK, please merge - after #1741 is addressed.

comment:13 Changed 8 years ago by jinmei

I believe you meant giving it to jelte:-)
changing.

comment:14 Changed 8 years ago by jinmei

  • Owner changed from jinmei to jelte

comment:15 Changed 8 years ago by jelte

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

Thanks, merged, closing ticket.

Note: See TracTickets for help on using tickets.