BIND 10 #2371: define dns::MasterLexer class

[BIND 10 Development]

#2371: define dns::MasterLexer class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121106
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  libdns++                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  loadzone-ng                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => jinmei


Comment:

 Hi Jinmei

 Here are my comments:

 * This can be removed from `master_lexer.cc` now I think:
 {{{
 -#include <sstream>
 }}}

 * I am trying to follow why the `END_OF_STREAM` definition became a
 duplicate for you. It worked here on Fedora GCC 4.7.2, so I assume this is
 a difference among compiler implementations. What compiler and version did
 you use to compile it? Also, does it have to be defined in
 `master_lexer_inputsource_unittest.cc`? That place seems wrong to me. Is
 it not because while there are protos everywhere, there's no actual
 storage allocation for it anywhere? Was it the compiler or linker which
 complained?
 * Does this work for you instead, without complaining about duplicates:
 {{{
 diff --git a/src/lib/dns/master_lexer_inputsource.cc
 b/src/lib/dns/master_lexer_inputsource.cc
 index f38d6c3..59952c6 100644
 --- a/src/lib/dns/master_lexer_inputsource.cc
 +++ b/src/lib/dns/master_lexer_inputsource.cc
 @@ -18,6 +18,8 @@ namespace isc {
  namespace dns {
  namespace master_lexer_internal {

 +const int InputSource::END_OF_STREAM = -1;
 +
  namespace { // unnamed namespace

  std::string
 diff --git a/src/lib/dns/master_lexer_inputsource.h
 b/src/lib/dns/master_lexer_inputsource.h
 index 9251bca..ba6e1e8 100644
 --- a/src/lib/dns/master_lexer_inputsource.h
 +++ b/src/lib/dns/master_lexer_inputsource.h
 @@ -38,7 +38,7 @@ namespace master_lexer_internal {
  class InputSource {
  public:
      /// \brief Returned by getChar() when end of stream is reached.
 -    static const int END_OF_STREAM = -1;
 +    static const int END_OF_STREAM;

      /// \brief Exception thrown when ungetChar() is made to go before
      /// the start of buffer.
 diff --git a/src/lib/dns/tests/master_lexer_inputsource_unittest.cc
 b/src/lib/dns/tests/master_lexer_inputsource_unittest.cc
 index f78edb5..8e0cf62 100644
 --- a/src/lib/dns/tests/master_lexer_inputsource_unittest.cc
 +++ b/src/lib/dns/tests/master_lexer_inputsource_unittest.cc
 @@ -27,10 +27,6 @@ using namespace std;
  using namespace isc::dns;
  using namespace isc::dns::master_lexer_internal;

 -// Some compilers cannot find symbols of class constants when used in the
 -// EXPECT_xxx macros, so we need explicit declaration.
 -const int InputSource::END_OF_STREAM;
 -
  namespace {

  class InputSourceTest : public ::testing::Test {
 }}}

 * The error condition reporting in `MasterLexer::pushSource()` seems
 inconsistent to me. For the two error conditions that may happen, we throw
 for one file-related problem, and we return `false` and an error string
 for another file-related problem, both in the same method to open a new
 file. Maybe we can simply document that it could throw
 `InputSource::OpenError` and let the caller handle both.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2371#comment:15>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development