BIND 10 #459: Dangerously counter-intuitive isc::auth::Query interface

[BIND 10 Development]

#459: Dangerously counter-intuitive isc::auth::Query interface
---------------------------+------------------------------------------------
      Reporter:  vorner    |        Owner:     
          Type:  defect    |       Status:  new
      Priority:  major     |    Milestone:     
     Component:  b10-auth  |   Resolution:     
      Keywords:            |    Sensitive:  0  
Estimatedhours:  0.0       |        Hours:  0  
      Billable:  1         |   Totalhours:  0  
      Internal:  0         |  
---------------------------+------------------------------------------------

Comment(by jinmei):

 Replying to [ticket:459 vorner]:
 > The Query constructor takes several const something& parameters. This
 allows for a temporary value to be passed as the parameter. And it is
 quite common to use such interface in this kind of manner.
 >
 > But the Query class stores const references and uses them later on. So,
 if the Query is use like this:
 >
 > {{{
 > Query mx_query(memory_datasrc, Name("mx.example.com"), RRType::MX(),
 response);
 > mx_query.process();
 > }}}
 [...]
 > This behaviour is counter-intuitive and isn't even mentioned in the
 documentation. I think it should be changed (either use non-const
 reference, pointer or make a copy of the parameters). If there really is
 reason for such strange expectation about the parameters, it should be
 documented at last.
 >
 I admit the interface is slippery.  The main reason for passing references
 is to avoid copy, and in the expected usage of query processing by
 b10-auth, the parameters should directly come from the question, which is
 expected to be valid until the query processing is completed.

 But since this interface is half open I agree it should be safer.  Some
 possibilities are:
  - make "Query" is just a function (at this moment there's not a strong
 reason it has to be a class)
  - likewise, we could merge the process() method in the Query constructor
  - pass pointers instead of references

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