224678 - nsComposerRegistration.cpp has suspicious code

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Martin Mokrejs]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030826
Build Identifier: 

Hi,
  I get the following with current snapshot on Tru64 Unix 5.1A:

cxx: Warning: nsComposerRegistration.cpp, line 59: integer conversion resulted
          in truncation
static NS_DEFINE_CID(kHTMLEditorCommandTableCID, NS_HTMLEDITOR_COMMANDTABLE_CID);
-------^
cxx: Warning: nsComposerRegistration.cpp, line 60: integer conversion resulted
          in truncation
static NS_DEFINE_CID(kHTMLEditorDocStateCommandTableCID,
NS_HTMLEDITOR_DOCSTATE_COMMANDTABLE_CID);
-------^
cxx: Warning: nsComposerRegistration.cpp, line 229: integer conversion resulted
          in truncation
    { "HTML Editor command table", NS_HTMLEDITOR_COMMANDTABLE_CID,
-----------------------------------^
cxx: Warning: nsComposerRegistration.cpp, line 233: integer conversion resulted
          in truncation
    { "HTML Editor doc state command table",
NS_HTMLEDITOR_DOCSTATE_COMMANDTABLE_CID,
---------------------------------------------^


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Martin Mokrejs]

Similarly, I get:

cxx: Warning: nsEditorRegistration.cpp, line 60: integer conversion resulted in
          truncation
static NS_DEFINE_CID(kEditorCommandTableCID, NS_EDITORCOMMANDTABLE_CID);
-------^
cxx: Warning: nsEditorRegistration.cpp, line 154: integer conversion resulted
          in truncation
    { "Editor Command Table", NS_EDITORCOMMANDTABLE_CID,
------------------------------^

Comment 2

[Malcolm Rowe]

*** Bug 224677 has been marked as a duplicate of this bug. ***

Comment 3

[Boris Zbarsky [:bzbarsky]]

Editor, not localization.  The problem, of course, is that the CID format used
in Mozilla is:

{ 0x00000000, 0x0000, 0x0000, { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 } }

while the CIDs in question use

{ 0x00000000, 0x0000, 0x0000, { 0x0000, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 } }

(so the first element of the sub-array does not fit in the PRUint8 it's assigned
into, and the last element does not exist).

I'm actually somewhat surprised this works... ;)

Comment 4

[David G King]

Sorry for spam, but I've given up on being surprised that a segment of code
works. But then, I'm fighting with the Microsoft Win32 API ;-)

Comment 5

[Martin Mokrejs]

Hi,
  any progress on this? ;)

Comment 6

[timeless]

Attachment: new cid

I'm tired of seeing a warning for this on windows. I've issued a new cid
because i don't want to rely on 'correcting' the old cid.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 143728 [details] [diff] [review]
new cid

If nobody uses the CID, why do we have it?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Meaning, why do we bother with an entry in the modules list if nobody ever does
a CreateInstance or a GetService for that CID / contractid (empty contractid).)

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 143728 [details] [diff] [review]
new cid

for lack of answer to my question, marking sr- to get off my review queue

Comment 10

[timeless]

well, the creature eventually leads to an nsIControllerCommandTable, i wonder if
there was an internal customer which had some use for it.

smfr: is there really no need for this cid and the constructor it chains to?
killing the pair would be nice...

Comment 11

[Simon Fraser [no longer active]]

I'd have to spend time looking around to see what the state of things there is,
time which I don't have right now.

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 143728 [details] [diff] [review]
new cid

Can we have a patch that removes this CID and its use at line 154 (remove the
nsModuleComponentInfo array element initializer)?

/be

Comment 13

[timeless]

Comment on attachment 143728 [details] [diff] [review]
new cid

it is used
consider:
Index: nsEditorRegistration.cpp
===================================================================
RCS file: /cvsroot/mozilla/editor/libeditor/build/nsEditorRegistration.cpp,v
retrieving revision 1.34
diff -u0 -r1.34 nsEditorRegistration.cpp
--- nsEditorRegistration.cpp
+++ nsEditorRegistration.cpp
@@ -56,0 +57 @@
+#if 0
@@ -60,0 +62 @@
+#endif
@@ -100 +102 @@
-
+#if 0
@@ -119 +121 @@
-
+#endif
@@ -153 +155 @@
-
+#if 0
@@ -157 +159 @@
-
+#endif

nsEditorRegistration.cpp
/mnt/ibm/mozhack/mozilla/editor/libeditor/build/nsEditorRegistration.cpp: In
function `nsresult nsEditorControllerConstructor(nsISupports *, const nsIID &,
void **)':
/mnt/ibm/mozhack/mozilla/editor/libeditor/build/nsEditorRegistration.cpp:87:
`kEditorCommandTableCID' undeclared (first use this function)

libeditor uses this CID to ensure service semantics and lifetime. It could
bloat itself with startup and shutdown stuff and a static critter which held a
reference, but i don't think that's a good tradeoff.

Comment 14

[timeless]

see http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTest/1080115920.23115.gz

Comment 15

[Simon Fraser [no longer active]]

@@ -56,0 +57 @@
+#if 0
@@ -60,0 +62 @@
+#endif
@@ -100 +102 @@
-
+#if 0
@@ -119 +121 @@
-
+#endif
@@ -153 +155 @@
-
+#if 0
@@ -157 +159 @@
-
+#endif

diff -u would be a little more useful here, timeless.

Comment 16

[timeless]

Comment on attachment 143728 [details] [diff] [review]
new cid

mozilla/editor/libeditor/build/nsEditorRegistration.cpp 	1.37

Comment 17

[Frank Wein [:mcsmurf]]

nsComposerRegistration.cpp needs to be fixed, too :)

Comment 18

[timeless]

Attachment: new CIDs

Comment 19

[timeless]

Comment on attachment 186541 [details] [diff] [review]
new CIDs

mozilla/editor/composer/src/nsComposerRegistration.cpp	1.20
mozilla/editor/composer/src/nsComposerRegistration.cpp	1.21

Comment 20

[Frank Wein [:mcsmurf]]

fixed now