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

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Daniel Holbert [:dholbert]]

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

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

This patch just makes us bail out if StartLoadingURL fails.

(We've got an opened file, so we have to close it first, though.)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix v1 w/ more context

same patch, just added more lines of context (so you can see where the file handle "fd" is normally opened / closed)

Comment 3

[Jason Duell]

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.

Comment 4

[Daniel Holbert [:dholbert]]

(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. :)

Comment 5

[Jason Duell]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e441f32d7eba

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e441f32d7eba