BIND 10 #2369: InputSource helper class for MasterLexer

[BIND 10 Development]

#2369: InputSource helper class for MasterLexer
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  muks
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121106
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  libdns++                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  loadzone-ng                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by muks):

 Hi Michal

 Replying to [comment:15 vorner]:
 > Hello
 >
 > Replying to [comment:14 muks]:
 > > > Also, in this ticket, I don't see the use of the `saveLine()`. Does
 it make any
 > > > sense to save the line and then rewind all the way to the beginning?
 Then the
 > > > line numbers will be wrong, won't they? Should the `saveLine()` and
 `compact()`
 > > > be a single method, so the `saveLine()` saves both the line and the
 position?
 > >
 > > Yes it would be a problem if we rewind to a position before where the
 saved line starts. IIRC we thought that as it's an internal class to
 `MasterLexer`, the lexer uses it knowing this behavior. In any case, it
 may be worth merging `saveLine()` and `compact()`, and perhaps call it
 something more meaningful like `mark()`.
 >
 > Yes, please. I see no use case for doing one without other, so they'd be
 called
 > both every time, which is long, or one of them forgotten, which would be
 wrong.

 I've added `InputSource::mark()`, but I've left `saveLine()` and
 `compact()` still available. This is because I don't know how the
 `MasterLexer` is going to use them.. if `mark()` is sufficient. Also the
 tests for these exist, and I want to be sure that these are not needed
 before removing them.

 > The file that does not exist, I guess ``/videokilledtheradiostar`` will
 never
 > exist there. But it might be confusing. I'd suggest something like
 > ``does-not-exist``, which is more obvious about the purpose, even while
 more conservative.

 Done. :)

 > I think the content of the string should be read from the actual file,
 so we don't have the same content twice:
 > {{{#!c++
 > TEST_F(InputSourceTest, file) {
 >     const char* str =
 >         ";; a simple (incomplete) zone file\n"
 >         "\n"
 >         "example.com. 3600 IN TXT \"test data\"\n"
 >         "www.example.com. 60 IN A 192.0.2.1\n"
 >         "www.example.com. 60 IN A 192.0.2.2\n";
 >     size_t str_length = strlen(str);
 >
 >     InputSource source(TEST_DATA_SRCDIR "/masterload.txt");
 >     checkGetAndUngetChar(source, str, str_length);
 > }
 > }}}

 Done. :)

 > Would it be possible to test the invalid read exception by some trick? I
 don't see one now (maybe something like a socket file could do the trick,
 but I'm not sure).

 I could not think of a way for it. I tried to make an `std::fstream` with
 output mode and passed it to the `InputSource`, but that just returned -1.
 I have seen both eof and fail bits set when a failure or EOF occurs. We
 check for eofbit first, but if a invalid read causes the eof bit to be
 set, this will return -1 and the exception will never be thrown. Actual
 EOF causes fail bit to be set too IIRC (in my box at least, which is why
 the eofbit is checked first).

 > And I'm thinking, what happens if we copy an opened file stream? Should
 we make the input source class non-copyable just to make sure? (well, with
 the const name, it is non-assignable at least, so for consistency too).

 Jinmei said somewhere that this must be copyable. I think he has just
 merged this branch into his `MasterLexer` where it's put into a
 `std::vector`, and that builds but I don't know yet what happens when the
 fstream gets copied.

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