458491 - [meta] possibly dead code (gcc warnings)

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[georgi - hopefully not receiving bugspam]

according to gcc build with -Wall -W there are:
13 unused vars
51 constant expressions

quick glance show they are plausible.

the most drastic is:
1601:../../../dist/include/lcms/lcms.h:1436: warning: comparison of unsigned
expression < 0 is always false

LCMS_INLINE void* _cmsMalloc(size_t size)
{
    if (size > ((size_t) 1024*1024*500)) return NULL;  // Never allow over
500Mb
********    if (size < 0) return NULL;********

probably the fix will be deleting stuff.

Comment 1

[georgi - hopefully not receiving bugspam]

Attachment: 51 possibly constant expressions

Comment 2

[georgi - hopefully not receiving bugspam]

Attachment: 13 possibly unused vars

Comment 3

[Jesse Ruderman]

You should probably file a bug for each affected file, like I did for bug 440771, so you can CC appropriate developers and attach patches.

Comment 4

[georgi - hopefully not receiving bugspam]

the ``unused'' are forked

Comment 5

[georgi - hopefully not receiving bugspam]

the forking bugspam of this bug is over.

Comment 6

[Arpad Borsos [:Swatinem]]

I don't think that its necessary to create a bug for every single warning that you get.
Better group them together according to the modules (like editor/, widget/ etc)
Also note that mozilla imports the sources of external projects into the source control.
Theora, LittleCMS and others. Would be better to file bugs upstream for those projects.

Comment 7

[John Daniels]

It seems like lines 1758 through 1775 on attachment one aren't valid, because type.TagPart() returns uint8, and the values of nsXPTType::T_I8 and so on are all less than 255 and greater than or equal to zero. Though if that's the case, I'm not sure why the warning is there at all.

Comment 8

[georgi - hopefully not receiving bugspam]

the uint8 warning puzzles me too.

Comment 9

[georgi - hopefully not receiving bugspam]

the uint8 warning is real on 64 bit system (and probably at least in one case on 32 bit systems).

the problem isn't the switch(), the problem is POPULATE macro:
http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/xpcconvert.cpp#1904


1904 #define POPULATE(_mode, _t)                                                  \
1905     PR_BEGIN_MACRO                                                           \
1906         cleanupMode = _mode;                                                 \
1907         if (capacity > ~(size_t)0 / sizeof(_t) ||


capacity is uint32 and ~(size_t)0 is uint64 and sizeof(_t) can't compensate for 32 bits.

this may be a bug of its own on 64 bit systems.

Comment 10

[:aceman]

Is there anything to do here? This looks like a meta bug now. If bug 458728 is closed, will this one be closed too?

Comment 11

[(dormant account)]

(In reply to :aceman from comment #10)
> Is there anything to do here? This looks like a meta bug now. If bug 458728
> is closed, will this one be closed too?

Once we know we have deleted all dead code, bug 458728 will close. This one only needs to close if we have deleted all of the dead code gcc knows about :)

Comment 12

[:aceman]

Haven't you mistaken bug 458728 for bug 457262? Or did I?

Comment 13

[(dormant account)]

(In reply to :aceman from comment #12)
> Haven't you mistaken bug 458728 for bug 457262? Or did I? 
Thanks, let me restate.

Once we know we have deleted all dead code, bug 457262 will close. This one only needs to close if we have deleted all of the dead code gcc knows about :)

Is there anything in particular you'd like to act upon related to these bugs?

Comment 14

[:aceman]

Maybe drive bug 458728 to closure?:)
My question was if that one is closed, if this one can be closed too, or if this one stays open so some new bugs can be linked to it.

Comment 15

[Nicholas Nethercote [inactive]]

All the bugs blocking this bug have been resolved, and it hasn't been modified since late 2011. I think we can safely close this one.