BIND 10 #2371: define dns::MasterLexer class
BIND 10 Development
do-not-reply at isc.org
Thu Nov 1 07:18:53 UTC 2012
#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's my review
* I think I'll add this to `master_lexer_inputsource.h` even though it's
not directly used there, as it probably belongs next to `InputSource` so
that it's not repeated elsewhere:
{{{
typedef boost::shared_ptr<InputSource> InputSourcePtr;
}}}
* I feel `MasterLexer::close()` should have something in its names to
indicate that it goes backward to the last source in the vector. Just
`close()` seems less, but I struggle to think of a good name for it (maybe
you did too). Maybe 'nestedOpen()`/`nestedClose()`? But only if you
agree.. it's just my opinion.
* Is it a valid situation where `getSourceName()` may be called on an
empty MasterLexer?
* I think "... MasterLexer will simply release.. " onwards can be changed
to just read "... MasterLexer will simply stop using the stream." To
release a reference may mean that a reference count is updated.
{{{
+ /// If it's a file, the opened file will be literally closed.
+ /// If it's a stream, \c MasterLexer will simply release the
reference to
+ /// the stream; the caller can assume it will be never used in
+ /// \c MasterLexer thereafter.
...
+ void close();
}}}
* This seems to be incorrect:
{{{
/// \return A string representation of the current source (see the
/// description)
size_t getSourceLine() const;
}}}
* I have pushed some minor comment updates. Please check them.
--
Ticket URL: <http://bind10.isc.org/ticket/2371#comment:9>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list