Closed Bug 132148 Opened 18 years ago Closed 18 years ago
Occurances of uninitialized variables being used before being set (in js/src/xpconnect)
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
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
You need to log in before you can comment on or make changes to this bug.