Closed Bug 837801 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: Document Navigation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attached patch fixSplinter Review
Attachment #709952 - Flags: review?(bugs)
Attachment #709952 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/60fccebd30e0
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.