854085 - error: variable 'rv' set but not used [-Werror=unused-but-set-variable] with gcc 4.7

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Mike Hommey [:glandium]]

nr_socket_prsock.cpp: In member function 'int mozilla::NrSocket::sendto(const void*, size_t, int, nr_transport_addr*)':
nr_socket_prsock.cpp:380:44: error: variable 'rv' set but not used [-Werror=unused-but-set-variable]
nr_socket_prsock.cpp: In member function 'int mozilla::NrSocket::recvfrom(void*, size_t, size_t*, int, nr_transport_addr*)':
nr_socket_prsock.cpp:407:44: error: variable 'rv' set but not used [-Werror=unused-but-set-variable]
nr_socket_prsock.cpp: In member function 'int mozilla::NrSocket::getaddr(nr_transport_addr*)':
nr_socket_prsock.cpp:430:44: error: variable 'rv' set but not used [-Werror=unused-but-set-variable]
nr_socket_prsock.cpp: In member function 'void mozilla::NrSocket::close()':
nr_socket_prsock.cpp:436:44: error: variable 'rv' set but not used [-Werror=unused-but-set-variable]
nriceresolver.cpp: In member function 'int mozilla::NrIceResolver::resolve(nr_resolver_resource*, int (*)(void*, nr_transport_addr*), void*, void**)':
nriceresolver.cpp:153:45: error: variable 'rv' set but not used [-Werror=unused-but-set-variable]
nriceresolver.cpp: In member function 'virtual nsresult mozilla::NrIceResolver::PendingResolution::OnLookupComplete(nsICancelable*, nsIDNSRecord*, nsresult)':
nriceresolver.cpp:183:41: error: variable 'rv' set but not used [-Werror=unused-but-set-variable]
nriceresolver.cpp: In static member function 'static int mozilla::NrIceResolver::cancel(void*, void*)':
nriceresolver.cpp:206:80: error: variable 'rv' set but not used [-Werror=unused-but-set-variable]

This can be tested on try with:

--- a/build/unix/mozconfig.linux
+++ b/build/unix/mozconfig.linux
@@ -1,4 +1,4 @@
 . "$topsrcdir/build/mozconfig.common"
 
-CC=/tools/gcc-4.5-0moz3/bin/gcc
-CXX=/tools/gcc-4.5-0moz3/bin/g++
+CC=/tools/gcc-4.7.2-0moz1/bin/gcc
+CXX=/tools/gcc-4.7.2-0moz1/bin/g++

Comment 1

[Mike Hommey [:glandium]]

Attachment: Fix unused-but-set-variable from ASSERT_ON_THREAD macro in media/mtransport/runnable_utils.h

Comment 2

[Dave Garrett]

Comment on attachment 728624 [details] [diff] [review]
Fix unused-but-set-variable from ASSERT_ON_THREAD macro in media/mtransport/runnable_utils.h

Wouldn't making 'rv' DebugOnly just make 'on' unused as well? Shouldn't ASSERT_ON_THREAD() just do nothing if not debug?

Comment 3

[Mike Hommey [:glandium]]

(In reply to Dave Garrett from comment #2)
> Comment on attachment 728624 [details] [diff] [review]
> Fix unused-but-set-variable from ASSERT_ON_THREAD macro in
> media/mtransport/runnable_utils.h
> 
> Wouldn't making 'rv' DebugOnly just make 'on' unused as well?

Strangely, gcc 4.7 doesn't complain.

> Shouldn't ASSERT_ON_THREAD() just do nothing if not debug?

Mmmmm probably.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Fix unused-but-set-variable from ASSERT_ON_THREAD macro in media/mtransport/runnable_utils.h

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 728643 [details] [diff] [review]
Fix unused-but-set-variable from ASSERT_ON_THREAD macro in media/mtransport/runnable_utils.h

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

r-.  we'll need to leave the RUN_ON_THREAD stuff to be on in production builds as webrtc thread assertions if violated can lead to security issues, and per discussions with dveditz we want to leave them on in opt/production builds "for a while" at least.

r+ without that change (i.e. r+ the previous patch).

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 728643 [details] [diff] [review]
Fix unused-but-set-variable from ASSERT_ON_THREAD macro in media/mtransport/runnable_utils.h

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

r+ actually - the assertions haven't been changed to be enabled opt/production yet, and thus the patch as it stands is correct.

Comment 7

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c7bf97d3b6f

Comment 8

[Mike Hommey [:glandium]]

Backout for bustage
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ea611c0295c

Comment 9

[Mike Hommey [:glandium]]

Relanded with the obvious fix

https://hg.mozilla.org/integration/mozilla-inbound/rev/58d7293faaff

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/58d7293faaff

Comment 11

[Eric Rescorla (:ekr)]

Attachment: Fix conditional for MOZ_ASSERT

Comment 12

[Eric Rescorla (:ekr)]

MOZ_ASSERT is conditional on DEBUG not MOZ_DEBUG

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/644e9c1856ce