837801 - nsSHistory.cpp:858:12: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Daniel Holbert [:dholbert]]

Build warning:
{
docshell/shistory/src/nsSHistory.cpp:858:12: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable]
}

For this line of code:
> 892   rv = GetCurrentURI(getter_AddRefs(currentURI));
https://mxr.mozilla.org/mozilla-central/source/docshell/shistory/src/nsSHistory.cpp#892

We don't bother checking 'rv' or 'currentURI' for failure at all, AFAIK.  It's been this way since this chunk of code was added, in 2005:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/docshell/shistory/src&command=DIFF_FRAMESET&file=nsSHistory.cpp&rev2=1.32&rev1=1.31

Elsewhere in this file, we call "GetCurrentURI" without bothering to capture the rv:
> 907   GetCurrentURI(getter_AddRefs(currentURI));
https://mxr.mozilla.org/mozilla-central/source/docshell/shistory/src/nsSHistory.cpp#907

We might as well just do that here, too. If we haven't bothered to check this nsresult for 7 years, and we don't even capture it at line 907, we might as well not capture it at line 892 either.

(alternately, we could add some error-checks, but that would change behavior & I'm not sure what the implications of that would be)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 2

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/60fccebd30e0

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/60fccebd30e0