[bind10-dev] Minor things about the master loader

[Michal 'vorner' Vaner]

Hello

On Mon, Nov 05, 2012 at 10:45:57AM -0800, JINMEI Tatuya / 神明達哉 wrote:
> > I have two questions about the currently being developed master loader.
> >  • Should we move it to a separate sub-namespace and directory
> >    (isc::dns::master_load)? There seems to be some number of classes already.
> 
> If you mean master_lexer_internal, it's a hack for hiding internal
> details as much as possible while they can be still tested (other than
> for tests we'd rather even hide them as a private and/or through
> pimpl).  For public classes like MasterLoader, at least it was not the
> original intend to introduce a separate namespace just for that.

I didn't mean master_lexer_internal (I didn't know it existed until now). I
really meant a new namespace, so we would have all classes related to loading
grouped together. I won't push for it much, just that it would look better to
me, with the number of classes.

> >    Also, the proposals seem to suggest there's a need for one ‒ for example
> >    #2376 suggests the struct Callbacks be part of dns::MasterLoader. But it
> >    could also be part of something else, like MasterLoaderContextBase. So I
> >    think we should put them on the same level (or all part of MasterLoader).
> 
> The background reason for separating the callback class/struct is to
> allow it to be used in rdata classes independently (as noted in
> #2376).  Wherever place is okay as long as we can reasonably achieve
> the goal.  But looking at it again, I now think neither MasterLoader
> nor MasterLoaderContext is a good place, because we'd then need to
> make rdata classes rely on the concept of master loader.  It seems to
> be awkward in that more primitive classes depend on higher level
> concepts.

That's why I'm creating it as a separate top-level class (struct) right now.
It's in the same file for now, but if that turns out to be a problem, it is easy
to move it elsewhere.

> >  • The InputSource provides line number only. But the API design suggests we
> >    should pass line number and byte offset in the file with errors and warnings.
> >    Should the master loader/lexer/whatever count the bytes as it calls
> >    inputSource.getChar() or should this function be added to the input source?
> 
> (in my memory) I though we'd like to have the byte count so we can
> implement the progress bar in the loadzone-ng (we could use line
> numbers, but to do so we need the total number of lines beforehand,
> which is not easy to count).  For that purpose the best place to do
> should be InputSource because that's the class that knows where
> exactly we are in the input file (or stream).

Therefore the InputSource needs to be extended. I don't know if Mukund already
merged it, though.

With regards

-- 
Something is obviously wrong. The thing works.

Michal 'vorner' Vaner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <https://lists.isc.org/pipermail/bind10-dev/attachments/20121105/7438a9b0/attachment.bin>