11666 - Eliminate plvector (was: [infinite loop] bugs - plvector.c)

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P3)

Description

[Jay Patel [:jay]]

Analysis of the code has determined possible infinite loop
or problem in the code below.  See bug
http://bugzilla.mozilla.org/show_bug.cgi?id=11588
for more information


plvector.c:289: warning: unsigned value >= 0 is always 1
plvector.c:311: warning: unsigned value >= 0 is always 1
plvector.c:312: warning: unsigned value >= 0 is always 1

Comment 1

[Wan-Teh Chang]

Reassigned bug to [e-mail],
the author of plvector.c.  The NSPR
group does not own this file.

Comment 2

[Warren Harris]

This bug should become: Eliminate plvector in favor of nsVector.

Cc'ing Brendan, and Norris (because I think this stuff was used by caps, but
maybe that's been fixed).

Comment 3

[Warren Harris]

Sorry, I said nsVector, but I meant nsVoidArray.

Comment 4

[Warren Harris]

Here's the list of nsVector (and indirectly plvector) users that should be
cleaned up:

nsVector

/editor/base/nsEditor.cpp, line 19 -- #include "nsVector.h"
/caps/include/nsCertificatePrincipal.h, line 21 -- #include "nsVector.h"
/caps/include/nsCertificatePrincipal.h, line 68 -- nsVector *
itsCertificateArray;
/caps/include/nsPrincipalArray.h, line 24 -- #include "nsVector.h"
/caps/include/nsPrincipalArray.h, line 63 -- nsVector * itsArray;
/caps/include/nsTarget.h, line 23 -- #include "nsVector.h"
/caps/include/nsPrivilegeManager.h, line 27 -- #include "nsVector.h"
/caps/src/nsPrincipalArray.cpp, line 39 -- this->itsArray = new nsVector();
/caps/src/nsPrincipalArray.cpp, line 75 -- if(!this->itsArray) this->itsArray =
new nsVector();
/caps/src/nsPrincipalArray.cpp, line 133 -- nsVector * in = new nsVector();
/caps/idl/nsITarget.idl, line 23 -- #include "nsVector.h"
/caps/idl/nsITarget.idl, line 24 -- typedef nsVector nsTargetArray;
/modules/oji/src/ProxyJNI.cpp, line 27 -- #include "nsVector.h"
/modules/oji/src/ProxyJNI.cpp, line 66 -- nsVector vec(0, 10);
/modules/oji/src/jvmmgr.cpp, line 414 --
JVM_GetJavaPrincipalsFromStackAsNSVector(JSStackFrame *pCurrentFrame)
/modules/oji/src/jvmmgr.cpp, line 467 -- ** nsVector. Use this vector to pass
into Tom's code to get to the JSPrinciapals
/modules/oji/src/jvmmgr.h, line 115 --
JVM_GetJavaPrincipalsFromStackAsNSVector(JSStackFrame *pCurrentFrame);
/modules/oji/src/lcglue.cpp, line 262 -- // TODO: Need raman's help. This needs
to convert between C++ [] array data type to a nsVector object.
/modules/oji/src/nsJNI.cpp, line 26 -- #include "nsVector.h"
/modules/oji/src/nsJNI.cpp, line 63 -- nsVector vec(10);
/modules/oji/src/nsJVMManager.cpp, line 274 -- fClassPathAdditions(new
nsVector()), fClassPathAdditionsString(NULL)
/modules/oji/src/nsJVMManager.h, line 34 -- #include "nsVector.h"
/modules/oji/src/nsJVMManager.h, line 225 -- nsVector* fClassPathAdditions;
/xpcom/components/nsServiceManager.cpp, line 20 -- #include "nsVector.h"
/xpcom/components/nsServiceManager.cpp, line 39 -- nsVector* mListeners; //
nsVector<nsIShutdownListener>
/xpcom/components/nsServiceManager.cpp, line 70 -- mListeners = new nsVector();
/xpcom/doc/TODO.html, line 150 -- <td>Replace use of nsVector (PL_Vector) with
nsISupportsArray</td>
/xpcom/ds/MANIFEST, line 27 -- nsVector.h
/xpcom/ds/Makefile.in, line 102 -- nsVector.h \
/xpcom/ds/makefile.win, line 53 -- nsVector.h \
/xpcom/ds/nsVector.h, line 19 -- #ifndef nsVector_h__
/xpcom/ds/nsVector.h, line 20 -- #define nsVector_h__
/xpcom/ds/nsVector.h, line 25 -- class nsVector : public PLVector {
/xpcom/ds/nsVector.h, line 28 -- nsVector(PRUint32 initialSize = 0, PRInt32
initialGrowBy = 0) {
/xpcom/ds/nsVector.h, line 31 -- ~nsVector(void) { PL_VectorFinalize(this); }
/xpcom/ds/nsVector.h, line 45 -- void Copy(nsVector* src, PRUint32 len,
PRUint32 dstPos = 0, PRUint32 srcPos = 0) {
/xpinstall/src/nsInstall.cpp, line 35 -- #include "nsVector.h"
/xpinstall/src/nsInstall.cpp, line 292 -- nsVector *paths = new nsVector();
/xpinstall/src/nsInstall.cpp, line 1213 -- mInstalledFiles = new nsVector();
/xpinstall/src/nsInstall.cpp, line 2087 -- nsInstall::ExtractDirEntries(const
nsString& directory, nsVector *paths)
/xpinstall/src/nsInstall.cpp, line 2142 -- nsInstall::DeleteVector(nsVector*
vector)
/xpinstall/src/nsInstall.h, line 39 -- #include "nsVector.h"
/xpinstall/src/nsInstall.h, line 263 -- nsVector* mInstalledFiles;
/xpinstall/src/nsInstall.h, line 287 -- PRInt32 ExtractDirEntries(const
nsString& directory, nsVector *paths);
/xpinstall/src/nsInstall.h, line 291 -- static void DeleteVector(nsVector*
vector);
/xpinstall/src/nsInstallTrigger.h, line 12 -- #include "nsVector.h"
/xpinstall/src/nsJSInstallTriggerGlobal.cpp, line 29 -- #include "nsVector.h"
/xpinstall/src/nsSoftwareUpdate.cpp, line 30 -- #include "nsVector.h"
/xpinstall/src/nsSoftwareUpdate.cpp, line 116 -- mJarInstallQueue = new
nsVector();
/xpinstall/src/nsSoftwareUpdate.h, line 12 -- #include "nsVector.h"
/xpinstall/src/nsSoftwareUpdate.h, line 75 -- nsVector* mJarInstallQueue;
/xpinstall/src/nsTopProgressNotifier.cpp, line 32 -- mNotifiers = new
nsVector();
/xpinstall/src/nsTopProgressNotifier.h, line 30 -- #include "nsVector.h"
/xpinstall/src/nsTopProgressNotifier.h, line 57 -- nsVector *mNotifiers;
/xpinstall/src/nsWinReg.h, line 34 -- #include "nsVector.h"
/xpinstall/src/nsXPITriggerInfo.h, line 29 -- #include "nsVector.h"
/xpinstall/src/nsXPITriggerInfo.h, line 96 -- nsVector mItems;
/xpinstall/src/nsXPInstallManager.cpp, line 27 -- #include "nsVector.h"
/dom/src/base/nsJSSecurityManager.cpp, line 554 -- void *javaPrincipals =
JVM_GetJavaPrincipalsFromStackAsNSVector(pFrameToStartLooking);
/webshell/embed/ActiveX/MozillaControl.dsp, line 4257 --
SOURCE=..\..\..\dist\include\nsVector.h

Cc'ing Kin for editor, Dan for xpinstall and Vidur for DOM.

Comment 5

[kinmoz]

Removed all references to nsVector.h from mozilla/editor/base.

Comment 6

[Warren Harris]

Dan, can you fix up xpinstall next? Thanks.

Comment 7

[Daniel Veditz [:dveditz]]

The three listed plvector lines are all asserts -- no danger of loops. I
converted xpinstall from nsVector to nsVoidArray anyway. Would have been nice
if all the equivalent methods weren't all given different names. Back to
warren to pick the next victim.

Comment 8

[Warren Harris]

Dan, do you have a list of how the method names differ between
PLVector/nsVector and nsVoidArray? That would help Norris... the next victim.

Comment 9

[Norris Boyd]

Fixed caps usage.

Forwarding to next component -- OJI.

Comment 10

[chris hofmann]

m12

Comment 11

[mark.lin]

I'm currently trying to fix it for the OJI module. Stay tuned.

Comment 12

[leger]

Resetting QA contact from leger.

Comment 13

[mark.lin]

O.K., I made the change for the OJI module. George, can you pass it on to the
next person?

Thanks.

Comment 14

[drapeau]

OJI fixed; forwarding to next component (XPCOM).

Comment 15

[Suresh Duddi (gone)]

we should eliminate use of nsVector right !

Comment 16

[Suresh Duddi (gone)]

I dont know exactly what to do about this bug. Could you figure out, do the
right thing, and pass it on to the next person in the list, I guess.

Some of these could be so so stale.

Comment 17

[Scott Collins]

All right.  I'm on it for XPCOM.

Comment 18

[Scott Collins]

changes are ready to go... just waiting on code reviews from dp (for
"nsServiceManager.cpp"), and brendan (for "nsAutoLock.cpp").

Comment 19

[Scott Collins]

Fixes checked in.  I think PLVector and nsVector are no longer used anywhere in
the app and can now be removed from the build.

Comment 20

[Paul Wyskoczka]

Verified per all the programmer's comments

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eba009c4e709
[components] For https://github.com/mozilla-mobile/android-components/issues/11666 - Add PendingUtils.defaultFlags helper
https://hg.mozilla.org/integration/autoland/rev/e64716764b60
[components] For https://github.com/mozilla-mobile/android-components/issues/11666 - Use PendingUtils.defaultFlags to specify mutability
https://hg.mozilla.org/integration/autoland/rev/ca0d780fe7d9
[fenix] For https://github.com/mozilla-mobile/fenix/issues/11666: TopSites layout from Constraint to Linear (https://github.com/mozilla-mobile/fenix/pull/11669)