BIND 10 #404: serialize RDATA for in memory data source

[BIND 10 Development]

#404: serialize RDATA for in memory data source
-------------------------------------+-------------------------------------
                 Reporter:  jinmei   |                Owner:  jinmei
                     Type:  task     |               Status:  reviewing
                 Priority:  major    |            Milestone:
                Component:  data     |  Sprint-20110419
  source                             |           Resolution:
                 Keywords:           |            Sensitive:  0
Estimated Number of Hours:  0.0      |  Add Hours to Ticket:  0
                Billable?:  1        |          Total Hours:  0
                Internal?:  0        |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:23 vorner]:

 > > I'm happy to raise this project wise, but I'm a bit pessimistic we can
 > > have a consensus.  For this specific case, as you seem to want to
 > > avoid hiding buffer quite strongly, and as I have to admit it wouldn't
 > > be very convincing to insist on this case while leaving many others,
 > > I'm okay with leaving the decision on this point to you.
 >
 > OK, I sent a mail to the dev list, let's at last try and wait a little
 bit. This task isn't in any real hurry anyway.

 Ack (but I'm okay with merging this anyway and creating a new task if
 the result of the dev list discussion suggests a change.  actually it
 may be better than holding off in terms of task progress management as
 long as we can keep track of the possibly open point).

 > Hmm, I see, I didn't think about this issue. Yes, it makes sense, I
 tried to hide it inside. But I still had to provide the getBuffer()
 method, because the MessageRenderer needs to store references to them from
 time to time. It could be refactored so it wouldn't need it, but it seemed
 too big change inside this ticket. Should I create a ticket for it?

 Yes, please.  I'm okay with moving forward with getBuffer() for now.

 > I changed it to void* and the field length function to return number of
 bytes. I also changed the getData to return void* and the constructor to
 take void* pointers to make it little bit more consistent and noted that
 it isn't actually expected that users would like to dig trough the data.

 Looks good, but we'll then need getFieldCount() in addition to
 getFieldDataSize() (btw, this one may better be named
 getFieldDataSpecSize() to be consistent with getFieldSpecData());
 otherwise the application cannot easily iterate over all fields using
 getFieldSpec() (it could still compute the # of fields by
 getFieldDataSpecSize() / sizeof(FieldSpec), but it's inconvenient and
 dangerous in that it relies too much on the internal representation).

 Some other comments to the revised code:

 - can't this be just a call to (Abstract)MessageRenderer::getLength()?
 {{{
 -    const size_t offset = buffer_.getLength();
 +    const size_t offset = getBuffer().getLength();
 }}}
 - Same for this:
 {{{
 +    virtual const void* getData() const { return (getBuffer().getData());
 }
 +    virtual size_t getLength() const { return (getBuffer().getLength());
 }
 }}}
  (actually shouldn't this be just removed because these methods of
  AbstractMessageRenderer aren't virtual?)
 - And same for these:
 {{{
 +        last_data_pos_ = getBuffer().getLength();
 ...
 +        if (getBuffer().getLength() == last_data_pos_) {
 ...
 +        fields_.back().len += getBuffer().getLength() - last_data_pos_;
 +        last_data_pos_ = getBuffer().getLength();
 }}}
 - This one can avoid the use of getBuffer() if we first dump the name
   to a local OutputBuffer and copy it using
   AbstraceMessageRenderer::writeData().  But that will be a bit more
   expensive due to the additional copy, so if you want to avoid this
   approach for this reason, I'm okay about it for now.
 {{{
 +        name.toWire(getBuffer());
 }}}

 > I hope I didn't forget anything.

 Except the small things mentioned above, it mostly looks okay.

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