TestProtocols.cpp:774:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable]

RESOLVED FIXED in mozilla13

Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks 1 bug)

Trunk
mozilla13
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Filing bug on this build warning:
{
TestProtocols.cpp: In function ‘nsresult TestProtocols::LoadURLsFromFile(char*)’:
TestProtocols.cpp:774:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable]
}

The only places this variable is touched are here:
> 772 nsresult LoadURLsFromFile(char *aFileName)
> 773 {
> 774     nsresult rv = NS_OK;

and here:
> 799                     rv = StartLoadingURL(urlString.get());
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/TestProtocols.cpp#772
Posted patch fix v1 (obsolete) — Splinter Review
This patch just makes us bail out if StartLoadingURL fails.

(We've got an opened file, so we have to close it first, though.)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #597969 - Flags: review?(jduell.mcbugs)
same patch, just added more lines of context (so you can see where the file handle "fd" is normally opened / closed)
Attachment #597969 - Attachment is obsolete: true
Attachment #597970 - Flags: review?(jduell.mcbugs)
Attachment #597969 - Flags: review?(jduell.mcbugs)
Comment on attachment 597970 [details] [diff] [review]
fix v1 w/ more context

Review of attachment 597970 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/test/TestProtocols.cpp
@@ +801,5 @@
> +                        // No need to log an error -- StartLoadingURL already
> +                        // did that for us, probably.
> +                        PR_Close(fd);
> +                        return rv;
> +                    }

A valiant attempt to add error checking to a fairly useless test program :)

None of the other calls to StartLoadingURL check for errors, and the program generally doesn't seem to do much error detection (ex. the caller of LoadURLsFromFile doesn't look at return val).  But there's no point soaking developer time into fixing all that right now, as  this program isn't run (only built) by buildbots anyway.  So this is fine.
Attachment #597970 - Flags: review?(jduell.mcbugs) → review+
(In reply to Jason Duell (:jduell) from comment #3)
> A valiant attempt to add error checking to a fairly useless test program :)

\o/
 
> None of the other calls to StartLoadingURL check for errors

Oh, true -- I thought I'd found one that did, but you're right.

> But there's no point soaking developer time into fixing all that right now
> [...] So this is fine.

cool, thanks. :)
Blocks: buildwarning
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/e441f32d7eba
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.