Issue 3029035: net: add DnsRRResovler to fetch arbitary DNS resource types.

Issue 3029035: net: add DnsRRResovler to fetch arbitary DNS resource types. (Closed)

Created:
10 years, 5 months ago by agl

Modified:
9 years, 7 months ago

Reviewers:
Mike Belshe, mbelshe, jar (doing other things)

CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org, Paweł Hajdan Jr.

Visibility:
Public.

Description

net: add DnsRRResovler to fetch arbitary DNS resource types. (Linux only for now.) TEST=net_unittests BUG=none

Patch Set 1 #

Total comments: 5

Created: 10 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+556 lines, -0 lines)			Patch
M	net/base/dns_util.h	View	2 chunks	+10 lines, -0 lines	0 comments	Download
A	net/base/dnsrr_resolver.h	View	1 chunk	+66 lines, -0 lines	0 comments	Download
A	net/base/dnsrr_resolver.cc	View	1 chunk	+324 lines, -0 lines	4 comments	Download
A	net/base/dnsrr_resolver_unittest.cc	View	1 chunk	+153 lines, -0 lines	1 comment	Download
M	net/net.gyp	View	2 chunks	+3 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Mike Belshe

lgtm - sorry for being slow. http://codereview.chromium.org/3029035/diff/1/3 File net/base/dnsrr_resolver.cc (right): http://codereview.chromium.org/3029035/diff/1/3#newcode95 net/base/dnsrr_resolver.cc:95: uint8 d = ...

10 years, 4 months ago (2010-08-02 14:18:28 UTC) #3

Mike Belshe

10 years, 4 months ago (2010-08-02 14:18:53 UTC) #4

On 2010/08/02 14:18:28, Mike Belshe wrote:
> lgtm - sorry for being slow.

I meant - LGTM with the style stuff cleaned up.

> 
> http://codereview.chromium.org/3029035/diff/1/3
> File net/base/dnsrr_resolver.cc (right):
> 
> http://codereview.chromium.org/3029035/diff/1/3#newcode95
> net/base/dnsrr_resolver.cc:95: uint8 d = *p;
> nit: I hate all these 1 char variable names!
> 
> http://codereview.chromium.org/3029035/diff/1/3#newcode99
> net/base/dnsrr_resolver.cc:99: if ((d & 0xc0) == 0xc0) {
> I'm sure 0xc0 is a mask of some sort; for those not intimately aware of the
> protocol, it's not readable :-(
> 
> http://codereview.chromium.org/3029035/diff/1/3#newcode100
> net/base/dnsrr_resolver.cc:100: if (jumps > 100)
> 100 is a hardcoded limit for ???
> 
> http://codereview.chromium.org/3029035/diff/1/3#newcode176
> net/base/dnsrr_resolver.cc:176: "\x7f\x00\x00\x01" /* 127.0.0.1 */, 4) == 0) {
> Is there any ipv6 issue here?  Maybe a TODO
> 
> http://codereview.chromium.org/3029035/diff/1/5
> File net/base/dnsrr_resolver_unittest.cc (right):
> 
> http://codereview.chromium.org/3029035/diff/1/5#newcode70
> net/base/dnsrr_resolver_unittest.cc:70: 0xce, 0xfe, 0x81, 0x80, 0x00, 0x01,
> 0x00, 0x02, 0x00, 0x06, 0x00, 0x01, 0x03,
> Holy smokes!
> 
> Since you managed to generate this somehow, maybe a comment about how the
packet
> was generated would suffice.

Expand Messages | Collapse Messages