Hi Pasi, On to, 2014-07-03 at 15:14 +0300, pasi.sjoh...@jolla.com wrote: > From: Pasi Sjöholm <pasi.sjoh...@jollamobile.com> > > If domain_append is set and forward_dns_reply() processes the response > for query without the domain name earlier than the response for one > with the domain name set we need to make sure that the response is > not sent back to the client if rcode and ancount are zero until the > last nameserver response is processed. > --- > src/dnsproxy.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/dnsproxy.c b/src/dnsproxy.c > index 7232b98..28e7cf7 100644 > --- a/src/dnsproxy.c > +++ b/src/dnsproxy.c > @@ -2068,7 +2068,8 @@ static int forward_dns_reply(unsigned char *reply, int > reply_len, int protocol, > } > > out: > - if (hdr->rcode > 0 && req->numresp < req->numserv) > + if ((hdr->rcode > 0 || (hdr->rcode == 0 && hdr->ancount == 0 && > + req->append_domain)) && req->numresp < req->numserv) > return -EINVAL; > > request_list = g_slist_remove(request_list, req);
Looks good, couple of comments - I sent a patch that changes how rcode values are compared, that patch does not touch this function thou. So if you could replace rcode 0 value with ns_r_noerror - Try to set the indentation of the second line so that it does not align with return statement, then it is easier to read the code, even better if the complex "if" statement could be written more clearly, you could probably even refactor it to two pieces - Add clarification to commit message about the "rcode == ns_r_noerror && ancount == 0" case, it is not very clear from looking the code when that situation happens. You had this explained partly in this mail thread already - Currently the commit message is just one big sentence, my head starts to hurt when reading it. Perhaps you could refactor it more. Cheers, Jukka _______________________________________________ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
