Closed
Bug 132148
Opened 23 years ago
Closed 23 years ago
Occurances of uninitialized variables being used before being set (in js/src/xpconnect)
Categories
(Core :: XPConnect, defect)
Tracking
()
VERIFIED
WONTFIX
People
(Reporter: mozilla-bugs, Assigned: dbradley)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•23 years ago
|
||
Reporter | ||
Comment 2•23 years ago
|
||
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...
Assignee | ||
Comment 3•23 years ago
|
||
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.
Reporter | ||
Comment 4•23 years ago
|
||
Wouldn't it be better to just initialize these things to something - this way we
can be sure...
Assignee | ||
Comment 5•23 years ago
|
||
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
Comment 6•23 years ago
|
||
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.
Assignee | ||
Comment 7•23 years ago
|
||
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: 23 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•