Issue 6180001: net: enable DnsRRResolver on Windows

Issue 6180001: net: enable DnsRRResolver on Windows (Closed)

Created:
9 years, 11 months ago by agl

Modified:
9 years, 7 months ago

Reviewers:
Mike Belshe, eroman

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

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

net: enable DnsRRResolver on Windows (Note that this code was developed by try-server so if something appears to be terribly wrong, it probably is.) BUG=none TEST=net_unittests

Patch Set 2 : ... #

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+125 lines, -11 lines)			Patch
M	build/common.gypi	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	net/base/dns_util.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	net/base/dnsrr_resolver.cc	View	1	3 chunks	+86 lines, -4 lines	0 comments	Download
M	net/base/dnsrr_resolver_unittest.cc	View		5 chunks	+36 lines, -7 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

eroman

http://codereview.chromium.org/6180001/diff/1/net/base/dnsrr_resolver.cc File net/base/dnsrr_resolver.cc (right): http://codereview.chromium.org/6180001/diff/1/net/base/dnsrr_resolver.cc#newcode270 net/base/dnsrr_resolver.cc:270: if (status) { Isn't this condition reversed? I thought ...

9 years, 11 months ago (2011-01-12 05:21:39 UTC) #4

agl

9 years, 11 months ago (2011-01-12 15:52:24 UTC) #5

http://codereview.chromium.org/6180001/diff/1/net/base/dnsrr_resolver.cc
File net/base/dnsrr_resolver.cc (right):

http://codereview.chromium.org/6180001/diff/1/net/base/dnsrr_resolver.cc#newc...
net/base/dnsrr_resolver.cc:270: if (status) {
On 2011/01/12 05:21:39, eroman wrote:
> Isn't this condition reversed? I thought success was going to be 0...

All the documentation says is "Returns success confirmation upon successful
completion". Empirically, however, it returns zero on success. I've changed the
condition to be "status != 0" to be clearer.

http://codereview.chromium.org/6180001/diff/1/net/base/dnsrr_resolver.cc#newc...
net/base/dnsrr_resolver.cc:296: rrdata += std::string(&len8, 1);
On 2011/01/12 05:21:39, eroman wrote:
> why not:
> rdata->append(len8);

I can't see an append(charT) method documented on std::string, but push_back
should work so I've changed it to that.

Expand Messages | Collapse Messages