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      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => muks


Comment:

 Hello

 '''General interface discussion''':

 From how the interface in the ticket looks like, it really means the
 internal
 storage should probably grow indefinitely. Or, well, see below for ideas.

 Anyway, is there any use for the `ungetAll()`? If it always rewinds to the
 start, maybe it is better to just create new `InputSource` in that case.

 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?

 '''Idea how to solve the growing''':

 The problem is with `ungetAll()`. As I mentioned above, I don't know if it
 is
 really needed. But the `ungetChar()` can have the parameter ‒ the
 character to
 return, then we could have buffer only for the returned characters.

 And, usually, we we'll be on top of files. Rewinding a file is possible ‒
 we
 just start reading it again. So we can have a special-case for the file
 version, that does not buffer, but rewinds the underlying file.

 '''Current code''':

 In the constructor, the `buffer_` is empty, isn't it? Therefore the
 `buffer_.size()` is zero. Is there a need to write the zero in that much
 complicated manner, the?
 {{{#!c++
 buffer_pos_(buffer_.size()),
 }}}

 Couldn't `!input_.good()` mean some other error as well?
 {{{#!c++
     int c = buffer_[buffer_pos_++];
     if (c == '\n') {
         line_++;
     }
 }}}

 What I mean is, if there's an I/O error, and it happens to be at the end
 of RR,
 we could load an incomplete zone, which is bad (IMO worse than failing to
 load).

 I think most of our exception classes are top-level ones (but I don't
 think
 there's anything forbidding having them nested). Anyway, there might be a
 better name than `UngetError`. Maybe `BeforeBeginning`?

 The relation between `ungetAll()` and `compact()` should be documented
 too.

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