Closed Bug 126473 Opened 18 years ago Closed 18 years ago

Occurances of uninitialized variables being used before being set (in extensions/xmlextras).

Categories

(Core :: XML, defect, P4)

x86
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla1.0

People

(Reporter: mozilla-bugs, Assigned: hjtoi-bugzilla)

References

Details

Attachments

(1 file, 2 obsolete files)

This bug is just for the warnings in various source files in
extensions/xmlextras/base/src. Currently
(http://tinderbox.mozilla.org/SeaMonkey/warn1014136200.29882.html - Tue, 19 Feb
2002 11:30 EST) TBox shows the following warnings:

extensions/xmlextras/base/src/nsDOMParser.cpp:368
 `class nsISupports * foundInterface' might be used uninitialized in this function

extensions/xmlextras/base/src/nsDOMSerializer.cpp:66
 `class nsISupports * foundInterface' might be used uninitialized in this function

extensions/xmlextras/base/src/nsXMLHttpRequest.cpp:156
 `class nsISupports * foundInterface' might be used uninitialized in this function
Bug 59652 is the meta-bug tracking the fight against these (potentially very
nasty) warnings.

P.S. Trying to make sure that 1.0 has as little warnings as possible.


Blocks: 59652
Keywords: mozilla1.0
Priority: -- → P4
I think the compiler is mistaken. The macro use:

NS_INTERFACE_MAP_BEGIN(nsDOMSerializer)
  NS_INTERFACE_MAP_ENTRY(nsISupports)
  NS_INTERFACE_MAP_ENTRY(nsIDOMSerializer)
  NS_INTERFACE_MAP_ENTRY_DOM_CLASSINFO(DOMSerializer)
NS_INTERFACE_MAP_END

expands (as far as I can tell) into 

NS_IMETHODIMP nsDOMSerializer::QueryInterface(REFNSIID aIID, void**
aInstancePtr)      
{                                                                             
  NS_ASSERTION(aInstancePtr,                                                  
               "QueryInterface requires a non-NULL destination!");            
  if ( !aInstancePtr )                                                        
    return NS_ERROR_NULL_POINTER;                                             
  nsISupports* foundInterface;
  if ( aIID.Equals(NS_GET_IID(nsISupports)) )                                  
    foundInterface = NS_STATIC_CAST(nsISupports*, this);                       
  else
  if ( aIID.Equals(NS_GET_IID(nsIDOMSerializer)) )                                  
    foundInterface = NS_STATIC_CAST(nsIDOMSerializer*, this);                       
  else
  if (aIID.Equals(NS_GET_IID(nsIClassInfo))) {                                
    foundInterface =                                                          
      nsContentUtils::GetClassInfoInstance(eDOMClassInfo_DOMSerializer_id);      
    NS_ENSURE_TRUE(foundInterface, NS_ERROR_OUT_OF_MEMORY);                   
                                                                              
    *aInstancePtr = foundInterface;                                           
                                                                              
    return NS_OK;                                                             
  } else
    foundInterface = 0;                                                       
  nsresult status;                                                            
  if ( !foundInterface )                                                      
    status = NS_NOINTERFACE;                                                 
  else                                                                        
    {                                                                         
      NS_ADDREF(foundInterface);                                             
      status = NS_OK;                                                         
    }                                                                         
  *aInstancePtr = foundInterface;                                             
  return status;                                                              
}

so foundInterface is set before use. Initializing it to 0 on declaration might
clear the warning, but that would be needless work. I don't have Linux available
to test, and the Windows build does not give that warning.
No, you got the code wrong. The actual code (as produced by doing c++ -E and
then pretty-printing by hand) is 

nsresult nsDOMSerializer::QueryInterface(const nsIID& aIID, void** aInstancePtr) {
   ;
   if ( !aInstancePtr ) return ((nsresult) 0x80004003L); 
   nsISupports* foundInterface;
   if ( aIID.Equals(nsCOMTypeInfo<nsISupports>::GetIID()) )
      foundInterface = static_cast< nsISupports* >(this);
   else if ( aIID.Equals(nsCOMTypeInfo<nsIDOMSerializer>::GetIID()) )
      foundInterface = static_cast< nsIDOMSerializer* >(this);
   else if (aIID.Equals(nsCOMTypeInfo<nsIClassInfo>::GetIID())) {
      static const nsCID kDOMSOF_CID = 
         { 0x9eb760f0, 0x4380, 0x11d2, {0xb3, 0x28, 0x00, 0x80, 0x5f, 0x8a,
0x38, 0x59} };
      nsCOMPtr<nsIDOMScriptObjectFactory> sof(do_GetService(kDOMSOF_CID));
      if (sof) {
         foundInterface = sof->GetClassInfoInstance(eDOMClassInfo_DOMSerializer_id);
         if (foundInterface) {
             *aInstancePtr = foundInterface; return 0;
         }
      }
   }
   else
      foundInterface = 0;
   nsresult status;
   if ( !foundInterface )
      status = ((nsresult) 0x80004002L);
   else {
      (foundInterface)->AddRef();
       status = 0;
   }
   *aInstancePtr = foundInterface;
   return status;
}

Note that if nsIClassInfo branch is chosen, but sof turnes out to be NULL,
foundInterface will indeed stay uninitialized...
Attached patch Proposed fix (obsolete) — Splinter Review
Thanks, I don't know how I got the expanded code I did. This fix should do it.
Attached patch This is the fix (obsolete) — Splinter Review
Argh, logic error in previous patch.
Attachment #74382 - Attachment is obsolete: true
Reviews?
Status: NEW → ASSIGNED
Whiteboard: [fixinhand]
Target Milestone: --- → mozilla1.0
Comment on attachment 74385 [details] [diff] [review]
This is the fix

This breaks compilation in layout/xul/base/src

make: Entering directory `/local/nogin/rpmBUILD/mozilla/layout/xul/base/src'
nsBoxObject.cpp
c++ -o nsBoxObject.o -c -DOSTYPE=\"Linux2.4\" -DOSARCH=\"Linux\" -DOJI
-D_IMPL_NS_HTML -I. -I./../../../base/src -I./../../../html/style/src
-I./../../../html/base/src -I./../../../html/forms/src -I./../../content/src
-I./../../../xml/content/src -I./../../../base/public 
-I../../../../dist/include/xpcom -I../../../../dist/include/string
-I../../../../dist/include/dom -I../../../../dist/include/locale
-I../../../../dist/include/content -I../../../../dist/include/gfx
-I../../../../dist/include/widget -I../../../../dist/include/view
-I../../../../dist/include/docshell -I../../../../dist/include/necko
-I../../../../dist/include/editor -I../../../../dist/include/htmlparser
-I../../../../dist/include/webshell -I../../../../dist/include/pref
-I../../../../dist/include/intl -I../../../../dist/include/gfx2
-I../../../../dist/include/imglib2 -I../../../../dist/include/unicharutil
-I../../../../dist/include/webbrwsr -I../../../../dist/include/xpconnect
-I../../../../dist/include/js -I../../../../dist/include/layout
-I../../../../dist/include
-I/usr/u/nogin/rpmsrc/BUILD.nuprl3/mozilla/dist/include/nspr
    -I/usr/X11R6/include   -fPIC  -I/usr/X11R6/include -fno-rtti
-fno-exceptions -Wall -Wconversion -Wpointer-arith -Wbad-function-cast
-Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -pedantic
-Wno-long-long -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -O 
-I/usr/X11R6/include -DMOZILLA_CLIENT -include ../../../../config-defs.h
-Wp,-MD,.deps/nsBoxObject.pp nsBoxObject.cpp
nsBoxObject.cpp: In method `nsresult nsBoxObject::QueryInterface (const
nsIID &, void **)':
nsBoxObject.cpp:72: no matching function for call to
`nsCOMPtr<nsIDOMScriptObjectFactory>::nsCOMPtr (const
nsGetServiceByCID, nsresult *)'
../../../../dist/include/xpcom/nsCOMPtr.h:514: candidates are:
nsCOMPtr<T>::nsCOMPtr () [with T =
nsIDOMScriptObjectFactory]
../../../../dist/include/xpcom/nsCOMPtr.h:521:		      
nsCOMPtr<T>::nsCOMPtr (const nsCOMPtr<T>
&) [with T = nsIDOMScriptObjectFactory]
../../../../dist/include/xpcom/nsCOMPtr.h:530:		      
nsCOMPtr<T>::nsCOMPtr (T *) [with T =
nsIDOMScriptObjectFactory]
../../../../dist/include/xpcom/nsCOMPtr.h:540:		      
nsCOMPtr<T>::nsCOMPtr (const
already_AddRefed<T> &) [with T = nsIDOMScriptObjectFactory]
../../../../dist/include/xpcom/nsCOMPtr.h:548:		      
nsCOMPtr<T>::nsCOMPtr (const
nsCOMPtr_helper &) [with T = nsIDOMScriptObjectFactory]
make: *** [nsBoxObject.o] Error 1
make: Leaving directory `/local/nogin/rpmBUILD/mozilla/layout/xul/base/src'
Attachment #74385 - Flags: needs-work+
Attachment attachment 74428 [details] [diff] [review] indeed fixes some of the warnings. It still leaves
one of the warnings though:

nsSchemaLoader.cpp: In method `nsresult nsSchemaLoader::ProcessFacet (nsSchema
*, nsIDOMElement *, nsIAtom *, nsISchemaFacet **)':
nsSchemaLoader.cpp:2672: warning: `PRUint16 facetType' might be used
uninitialized in this function



I filed bug 131347 on that because I don't know what the default value should be.
Comment on attachment 74428 [details] [diff] [review]
Ba-ad day! I swear this is the one

r=peterv
Attachment #74428 - Flags: review+
Comment on attachment 74428 [details] [diff] [review]
Ba-ad day! I swear this is the one

>+    nsresult rv;                                                           \
>+    nsCOMPtr<nsIDOMScriptObjectFactory> sof(do_GetService(kDOMSOF_CID, &rv));\

Split this (and the other one too) line into two lines after the comma so that
the rhs of the macro lines up nicely.

Other than that, sr=jst
Attachment #74428 - Flags: superreview+
Will do, I have that fixed in my tree. Sending for approval...
Comment on attachment 74428 [details] [diff] [review]
Ba-ad day! I swear this is the one

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #74428 - Flags: approval+
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [fixinhand]
Verifying, currently
(http://tinderbox.mozilla.org/SeaMonkey/warn1016552220.1515.html - Tue, 19 Mar
2002 10:37 EST) the only "xxx might be used uninitialized" warning in xmlextras
is the nsSchemaLoader.cpp one that is covered in a separate bug 127490.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.