fix uninitialized variable warning for vs2012 compilation

fix uninitialized variable warning for vs2012 compilation

NOTRY=true
R=rsleevi@chromium.org
BUG=155147, 143646, 155110
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=161781

[Ryan Sleevi, 2012-10-12 19:35:45]

LGTM

https://codereview.chromium.org/11128003/diff/1/net/http/http_request_headers.cc
File net/http/http_request_headers.cc (right):

https://codereview.chromium.org/11128003/diff/1/net/http/http_request_headers...
net/http/http_request_headers.cc:211: const base::ListValue* header_list = NULL;
Pretty sure this is just a bug in MSVC's static analysis, since the logic on
213-217 should always short-circuit, should it not?

The only nit I'd have is that if we're going to go this route of pedantry, it
would be nice to keep symmetry, and ensure the dict is also NULL initialized.

[scottmg, 2012-10-12 19:40:43]

On 2012/10/12 19:35:45, Ryan Sleevi wrote:
> LGTM
> 
>
https://codereview.chromium.org/11128003/diff/1/net/http/http_request_headers.cc
> File net/http/http_request_headers.cc (right):
> 
>
https://codereview.chromium.org/11128003/diff/1/net/http/http_request_headers...
> net/http/http_request_headers.cc:211: const base::ListValue* header_list =
NULL;
> Pretty sure this is just a bug in MSVC's static analysis, since the logic on
> 213-217 should always short-circuit, should it not?

Yes, as far as I can tell looking through the layers of Get() it is a
mis-diagnostic.

> 
> The only nit I'd have is that if we're going to go this route of pedantry, it
> would be nice to keep symmetry, and ensure the dict is also NULL initialized.

I can initialize them both or I'm happy to disable the warning in net/net.gyp if
you prefer the decrease the noise that way.

[Ryan Sleevi, 2012-10-12 19:58:09]

On 2012/10/12 19:40:43, scottmg wrote:
> On 2012/10/12 19:35:45, Ryan Sleevi wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/11128003/diff/1/net/http/http_request_headers.cc
> > File net/http/http_request_headers.cc (right):
> > 
> >
>
https://codereview.chromium.org/11128003/diff/1/net/http/http_request_headers...
> > net/http/http_request_headers.cc:211: const base::ListValue* header_list =
> NULL;
> > Pretty sure this is just a bug in MSVC's static analysis, since the logic on
> > 213-217 should always short-circuit, should it not?
> 
> Yes, as far as I can tell looking through the layers of Get() it is a
> mis-diagnostic.
> 
> > 
> > The only nit I'd have is that if we're going to go this route of pedantry,
it
> > would be nice to keep symmetry, and ensure the dict is also NULL
initialized.
> 
> I can initialize them both or I'm happy to disable the warning in net/net.gyp
if
> you prefer the decrease the noise that way.

I'd rather pedantry and keeping the warning on (to catch real bugs) than
disabling it. I just get bothered by initialization asymmetry, especially in
"obviously correct" code like this.

[scottmg, 2012-10-12 20:00:48]

On 2012/10/12 19:58:09, Ryan Sleevi wrote:
> On 2012/10/12 19:40:43, scottmg wrote:
> > On 2012/10/12 19:35:45, Ryan Sleevi wrote:
> > > LGTM
> > > 
> > >
> >
>
https://codereview.chromium.org/11128003/diff/1/net/http/http_request_headers.cc
> > > File net/http/http_request_headers.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/11128003/diff/1/net/http/http_request_headers...
> > > net/http/http_request_headers.cc:211: const base::ListValue* header_list =
> > NULL;
> > > Pretty sure this is just a bug in MSVC's static analysis, since the logic
on
> > > 213-217 should always short-circuit, should it not?
> > 
> > Yes, as far as I can tell looking through the layers of Get() it is a
> > mis-diagnostic.
> > 
> > > 
> > > The only nit I'd have is that if we're going to go this route of pedantry,
> it
> > > would be nice to keep symmetry, and ensure the dict is also NULL
> initialized.
> > 
> > I can initialize them both or I'm happy to disable the warning in
net/net.gyp
> if
> > you prefer the decrease the noise that way.
> 
> I'd rather pedantry and keeping the warning on (to catch real bugs) than
> disabling it. I just get bothered by initialization asymmetry, especially in
> "obviously correct" code like this.

Done.

[commit-bot: I haz the power, 2012-10-12 20:29:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/11128003/5001

[commit-bot: I haz the power, 2012-10-13 16:52:22]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2012-10-13 21:57:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/11128003/5001

[commit-bot: I haz the power, 2012-10-13 21:57:30]

Change committed as 161781