[bind10-dev] BIND 10 trac503, updated. implement ANY query logic

[Shane Kerr]

Michal,

On Wed, 2011-01-26 at 09:36 +0100, Michal 'vorner' Vaner wrote:
> On Wed, Jan 26, 2011 at 06:49:18AM +0000, BIND 10 source code commits wrote:
> > +    const bool qtype_is_any = (qtype_ == RRType::ANY());
> > +    RRsetList result_rrsets;
> > +    RRsetList* target = NULL;
> > +
> >      response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
> >      const MemoryDataSrc::FindResult result =
> >          memory_datasrc_.findZone(qname_);
> > @@ -141,20 +145,33 @@ Query::process() const {
> >      response_.setHeaderFlag(Message::HEADERFLAG_AA);
> >      while (keep_doing) {
> >          keep_doing = false;
> > -        Zone::FindResult db_result = result.zone->find(qname_, qtype_);
> > +        if (qtype_is_any) {
> > +            target = &result_rrsets;
> > +        }
> 
> This just got my attention. Not that it would be a problem, just a small
> suggestion. This way the RRsetList is created each time. It probably isn't much,
> but ANY queries are rare.
> 
> I would suggest using an
> std::auto_ptr<RRsetList> target(qtype_is_any ? new RRSetList : NULL)
> 
> This way the RRsetList is created only when needed, the code complexity doesn't
> grow and the auto_ptr is simple enough to be eliminated completely in
> optimisations.
> 
> What do you think?

Well... if auto_ptr is indeed eliminated with optimization then this
makes sense. Otherwise we're just trading one constructor for
another. :)

A quick check using "g++ -S" and looking at a short program seems to
indicate that g++ is smart enough to optimize unused auto_ptr away, at
least in trivial cases.

So, I agree the code is simpler so we should probably go this way!

--
Shane