Closed Bug 132148 Opened 18 years ago Closed 18 years ago

Occurances of uninitialized variables being used before being set (in js/src/xpconnect)

Categories

(Core :: XPConnect, defect)

x86
Linux
defect
Not set

Tracking

()

VERIFIED WONTFIX

People

(Reporter: mozilla-bugs, Assigned: dbradley)

References

Details

Attachments

(1 file)

This bug is just for the "xxx might be used uninitialized" warnings in various
source files in js/src/xpconnect directory.
Currently (http://tinderbox.mozilla.org/SeaMonkey/warn1016552220.1515.html -
Tue, 19 Mar 2002 10:37 EST) TBox shows the following warnings:

js/src/xpconnect/src/xpccomponents.cpp:1678
 `nsresult res' might be used uninitialized in this function

js/src/xpconnect/src/xpcconvert.cpp:1304
 `nsresult rv' might be used uninitialized in this function

js/src/xpconnect/src/xpcconvert.cpp:1305
 `double number' might be used uninitialized in this function

js/src/xpconnect/src/xpcconvert.cpp:1632
 `enum XPCConvert::JSArray2Native(XPCCallContext &, void **, long int, unsigned
int, unsigned int, const nsXPTType &, int, const nsID *, uintN *)::CleanupMode
cleanupMode' might be used uninitialized in this function

js/src/xpconnect/src/xpcconvert.cpp:1636
 `JSUint32 initedCount' might be used uninitialized in this function

js/src/xpconnect/src/xpcconvert.cpp:665
 `const PRUnichar * chars' might be used uninitialized in this function

js/src/xpconnect/src/xpcconvert.cpp:668
 `PRUint32 length' might be used uninitialized in this function

js/src/xpconnect/src/xpcwrappedjsclass.cpp:801
 `jsval * stackbase' might be used uninitialized in this function

js/src/xpconnect/src/xpcwrappedjsclass.cpp:810
 `void (* older)(JSContext *, const char *, JSErrorReport *)' might be used
uninitialized in this function

js/src/xpconnect/src/xpcwrappedjsclass.cpp:811
 `JSBool success' might be used uninitialized in this function

js/src/xpconnect/src/xpcwrappednative.cpp:1627
 `uint8 paramCount' might be used uninitialized in this function

js/src/xpconnect/src/xpcwrappednativeinfo.cpp:298
 `struct JSString * str' might be used uninitialized in this function

js/src/xpconnect/src/xpcwrappednativejsops.cpp:256
 `const char * name' might be used uninitialized in this function

js/src/xpconnect/src/xpcwrappednativejsops.cpp:897
 `class XPCWrappedNative * oldResolvingWrapper' might be used uninitialized in
this function

js/src/xpconnect/src/xpcwrappednativeproto.cpp:172
 `class ClassInfo2WrappedNativeProtoMap * map' might be used uninitialized in
this function

js/src/xpconnect/src/xpcwrappednativeproto.cpp:173
 `struct XPCLock * lock' might be used uninitialized in this function
Bug 59652 is the meta-bug tracking the fight against these (potentially very
nasty) warnings; bug 59659 is for all JS warnings.
Blocks: 59652, 59659
Some of these appear to be real bugs. For example, the `JSUint32 initedCount' in
xpcconvert.cpp:1636  There if the Alloc in the POPULATE macro fails, the code
might end up doing a for loop from 0 to initedCount w/o ever setting initedCount
to anything...
It's actually ok, since this code will only get executed if 'array' is non-null.
'array' can only be non-null if POPULATE macro was invoked and that initializes
the other variables. Such initializations when not needed add time to the
function. It might be ok to initialize these in the error case to quiet the
compiler, if that works.

I'll go through and double check the rest to be sure. I did this a while back,
like a year ago. That's why I haven't jumped on this earlier. It's worth a
second look, though.
Wouldn't it be better to just initialize these things to something - this way we
can be sure...
The use of str in this case I believe could be a problem.

I checked the other instances. They are all initialized for all paths to the
use.

We could initialize them to quiet the warnings, but that would impact
performance. Many of these functions are called many times, so there could be a
slight performance hit, especially in combination.

We could create a conditional initialization macro such that it initializes in
debug mode, quieting these warnings, but not initialized in release. Something
along the lines like below.

#ifdef DEBUG
#define NS_DECLARE(type, name, value) type name = value;
#else
#define NS_DECLARE(type, name, value) type name;
#endif
dbradley: Make that change if you want. But, I believe, you are just going to
end up with the same sort of warning about interfaceName. The current code
'uses' str only in the sense that STRING_TO_JSVAL twiddles a few bits from it to
set interfaceName. Whether it was initialized of not makes no material differece
here. But, I don't see anything wrong with your change per se.

I think that the macro scheme you suggest is a bad idea... It is harder to read
than plain C/C++ - and thus easier to do wrong and misinterpret. Just one
instance of misuse could make a critical - and hard to find - bug that would act
one way in debug and another in release.

I hadn't made the change to solely quiet the warning, but to address the
uninitialized use, but as you pointed out is also harmless.

I do think there is some value in quieting these warnings, as they hide real
uninitialized variables cases. I hadn't thought about misuing the macro, was
thinking its use would be solely confined to quieting such warnings. So the
ugliness and risk of misuse probably outweigh its use.

I'm marking won't fix, as all cases are of no real danger. If someone can get
some stats showing that the potential performance hit is trivial and justified,
reopen it.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WONTFIX
Marking Verified -
Status: RESOLVED → VERIFIED
Blocks: 179819
You need to log in before you can comment on or make changes to this bug.