Opened 8 years ago

Closed 8 years ago

#1857 closed defect (complete)

make isc::config::ModuleSpecError a derived class of isc::Exception

Reported by: jinmei Owned by: jelte
Priority: medium Milestone: Sprint-20120515
Component: configuration Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 2 Add Hours to Ticket: 0
Total Hours: 0.27 Internal?: no


Currently it's a direct derived class of std::exception:

    /// A standard ModuleSpec exception that is thrown when a
    /// specification is not in the correct form.
    /// TODO: use jinmei's exception class as a base and not c_str in
    /// what() there
    class ModuleSpecError : public std::exception {
        ModuleSpecError(std::string m = "Module specification is invalid") : msg(m) {}
        ~ModuleSpecError() throw() {}
        const char* what() const throw() { return (msg.c_str()); }
        std::string msg;

As noted in the comment we should make it a derived class of
isc::Exception. We often consider other std::exception outside of
isc::Exception as a fatal error, so this can affect stability in case
an error happens.

Implementation-wise, this should be a trivial cleanup.


Change History (8)

comment:1 Changed 8 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20120501

comment:2 Changed 8 years ago by jelte

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

comment:3 Changed 8 years ago by jelte

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


BTW, I noticed a few other cases where our exceptions are based on std::exception, should we fix those too?

./src/lib/util/unittests/resolver.h:        class NoSuchRequest : public std::exception { };
./src/lib/util/unittests/resolver.h:        class DifferentRequest : public std::exception { };
./src/lib/log/message_exception.h:class MessageException : public std::exception {

comment:4 Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:5 Changed 8 years ago by vorner

  • Owner changed from vorner to jelte


The cleanup looks OK. And I think the change is small enough, so we can piggy-back changes of all the exceptions (they should be fixed and I think this ticket is good enough opportunity).

comment:6 Changed 8 years ago by jelte

  • Owner changed from jelte to vorner

Ok, did MessageException? as well. For readability, i added an isc_throw_2 (similar to isc_throw_1 but with 2 params)

I left out the two in lib/util/unittests/resolver.h, those seemed a bit internal

comment:7 Changed 8 years ago by vorner

  • Owner changed from vorner to jelte
  • Total Hours changed from 0 to 0.27

OK. I think it is ready for merge.

comment:8 Changed 8 years ago by jelte

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

Thanks, merged, closing ticket.

Note: See TracTickets for help on using tickets.