120045 - Multi-character character constants in nsExternalHelperAppService.cpp

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P3)

Description

[Joseph]

while compiling, I noticed a bunch of multi-character character constants.
Upon further examination, I found that
uriloader/exthander/nsExternalHelperAppService.cpp had things like 'TEXT' in it.
 I modified them the single-quotes to double-quotes where appropriate, and the
patch I'll post shortly will be the result.

Comment 1

[Joseph]

Attachment: result of grep -r "'TEXT'" *

OK.  It's not a patch.	It turned out that there're a lot of the buggers out
there.	I'm going to assume you auto-generate the 'TEXT' and 'ttxt' type stuff.
 This is a grep -r "'TEXT'" instead.  There are other single-quote strings, see
the file I mentioned for a partial list.

I know how easy it is to get the single and double quotes mixed up, especially
when switching between perl, sh, or other scripting and C/C++.	;)

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

->Networking

Comment 3

[Kathleen :Brade]

I believe this bug is invalid.
Every single case in the attached file is for Mac purposes; the Mac expects some
items (like file types) to use single quotes so it can build as a long rather
than as a string.

Comment 4

[Joseph]

Intersting note on the Mac builds.
Still, not all files referenced were related to Macs (I first noticed the error
"nsXMLMIMEDataSource.cpp:419: warning: multi-character character constant" under
Linux.  I see it under Solaris as well.)

The files I see throwing up these errors are:
  nsXMLMIMEDataSource.cpp
  nsExternalHelperAppService.cpp


The exact errors from these files are:

 10992  /usr/local/bin/g++ -o nsXMLMIMEDataSource.o -c -DOSTYPE=\"SunOS5\"
-DOSARCH=\"SunOS\" -DOJI -D_IMPL_N
 10992  S_NET  -I../../../dist/include/xpcom -I../../../dist/include/string
-I../../../dist/include/necko -I.
 10992  ./../../dist/include/pref -I../../../dist/include/mimetype
-I../../../dist/include -I/usr/local/src/b
 10992  uilding/moz/mozilla/dist/include/nspr      -I/usr/openwin/include  
-fPIC  -I/usr/openwin/include -fn
 10992  o-rtti -fno-exceptions -pedantic -Wno-long-long -pthreads  -DNDEBUG
-DTRIMMED -O2 -pipe -fexpensive-o
 10992  ptimizations -fdelayed-branch -fschedule-insns2 -msupersparc 
-I/usr/openwin/include -DMOZILLA_CLIENT
 10992   -include ../../../config-defs.h -Wp,-MD,.deps/nsXMLMIMEDataSource.pp
nsXMLMIMEDataSource.cpp
 10993  nsXMLMIMEDataSource.cpp: In method `nsresult
nsXMLMIMEDataSource::InitFromHack()':
 10994  nsXMLMIMEDataSource.cpp:419: warning: multi-character character constant
 10995  nsXMLMIMEDataSource.cpp:419: warning: multi-character character constant
 10996  nsXMLMIMEDataSource.cpp:458: warning: multi-character character constant
 10997  nsXMLMIMEDataSource.cpp:458: warning: multi-character character constant
 10998  nsXMLMIMEDataSource.cpp:461: warning: multi-character character constant
 10999  nsXMLMIMEDataSource.cpp:461: warning: multi-character character constant
 11000  nsXMLMIMEDataSource.cpp:474: warning: multi-character character constant
 11001  nsXMLMIMEDataSource.cpp:474: warning: multi-character character constant
 11002  nsXMLMIMEDataSource.cpp:478: warning: multi-character character constant
 11003  nsXMLMIMEDataSource.cpp:478: warning: multi-character character constant
 11004  nsXMLMIMEDataSource.cpp:481: warning: multi-character character constant
 11005  nsXMLMIMEDataSource.cpp:481: warning: multi-character character constant
 11006  nsXMLMIMEDataSource.cpp:491: warning: multi-character character constant
 11007  nsXMLMIMEDataSource.cpp:491: warning: multi-character character constant
 11008  nsXMLMIMEDataSource.cpp:494: warning: multi-character character constant
 11009  nsXMLMIMEDataSource.cpp:494: warning: multi-character character constant
 11010  nsXMLMIMEDataSource.cpp:535: warning: multi-character character constant
 11011  nsXMLMIMEDataSource.cpp:535: warning: multi-character character constant

and

   11391  /usr/local/bin/g++ -o nsExternalHelperAppService.o -c
-DOSTYPE=\"SunOS5\" -DOSARCH=\"SunOS\" -DOJI  -
 11391  I../../dist/include/xpcom -I../../dist/include/string
-I../../dist/include/unicharutil -I../../dist/i
 11391  nclude/mimetype -I../../dist/include/uriloader -I../../dist/include/dom
-I../../dist/include/necko -I
 11391  ../../dist/include/rdf -I../../dist/include/webshell
-I../../dist/include/helperAppDlg -I../../dist/i
 11391  nclude/plugin -I../../dist/include/pref -I../../dist/include/exthandler
-I../../dist/include -I/usr/l
 11391  ocal/src/building/moz/mozilla/dist/include/nspr     
-I/usr/openwin/include   -fPIC  -I/usr/openwin/i
 11391  nclude -fno-rtti -fno-exceptions -pedantic -Wno-long-long -pthreads 
-DNDEBUG -DTRIMMED -O2 -pipe -fe
 11391  xpensive-optimizations -fdelayed-branch -fschedule-insns2 -msupersparc 
-I/usr/openwin/include -DMOZI
 11391  LLA_CLIENT -include ../../config-defs.h
-Wp,-MD,.deps/nsExternalHelperAppService.pp nsExternalHelperA
 11391  ppService.cpp
 11392  nsExternalHelperAppService.cpp:93: warning: multi-character character
constant
 11393  nsExternalHelperAppService.cpp:93: warning: multi-character character
constant
 11394  nsExternalHelperAppService.cpp:94: warning: multi-character character
constant
 11395  nsExternalHelperAppService.cpp:94: warning: multi-character character
constant
 11396  nsExternalHelperAppService.cpp:95: warning: multi-character character
constant
 11397  nsExternalHelperAppService.cpp:95: warning: multi-character character
constant
 11398  nsExternalHelperAppService.cpp:96: warning: multi-character character
constant
 11399  nsExternalHelperAppService.cpp:96: warning: multi-character character
constant
 11400  nsExternalHelperAppService.cpp:97: warning: multi-character character
constant
 11401  nsExternalHelperAppService.cpp:97: warning: multi-character character
constant
 11402  nsExternalHelperAppService.cpp:98: warning: multi-character character
constant
 11403  nsExternalHelperAppService.cpp:98: warning: multi-character character
constant
 11404  nsExternalHelperAppService.cpp:100: warning: multi-character character
constant
 11405  nsExternalHelperAppService.cpp:100: warning: multi-character character
constant
 11406  nsExternalHelperAppService.cpp:102: warning: multi-character character
constant
 11407  nsExternalHelperAppService.cpp:102: warning: multi-character character
constant
 11408  nsExternalHelperAppService.cpp:103: warning: multi-character character
constant
 11409  nsExternalHelperAppService.cpp:103: warning: multi-character character
constant
 11410  nsExternalHelperAppService.cpp:104: warning: multi-character character
constant
 11411  nsExternalHelperAppService.cpp:104: warning: multi-character character
constant
 11412  nsExternalHelperAppService.cpp:112: warning: multi-character character
constant
 11413  nsExternalHelperAppService.cpp:112: warning: multi-character character
constant


I'm reopening.  If these files are related only to Macs, then something's
severely gronky, since I'm seeing the messages on Linux and Solaris builds.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Those files are XP.  The problem with attempting to #ifdef XP_MAC those parts of
the code is that there is no way to #ifdef out the parts of the IDL that refer
to type and creator.

Bill, would it be ok to leave type and creator not filled in on non-mac builds?
 Maybe make the getters and setters for type/creator assert and/or fail on
non-mac builds?

Comment 6

[Bill Law]

That sounds OK, but I don't think I can say for sure.  You should ask mscott
since he set this up originally.

Comment 7

[Scott MacGregor]

And we think this is an interesting problem to solve because? (i.e. what's the
impact of not changing anything here? Just wondering.....we've got a lot of
important stuff to get done before 1.0)....

Comment 8

[Joseph]

Because I mistakenly thought it was a coding error.  I didn't know it was Mac
weirdness leaking through into others' code.

It's not important if nobody will ever touch that code.  It's definitely
something to look into fixing later, since IMHO it could cause problems down the
road if anyone tries to look at the strings that aren't (difference between a
pointer and a char is vast).

If it doesn't break anything now, <i>meinetwegen</i> it can go to LATER.

Comment 9

[Boris Zbarsky [:bzbarsky]]

I'll take this, but not happening any time soon...

Comment 10

[Joseph]

That's alright.  It's not as bad as I had thought.  I also knocked the priority
down to minor.

Since it's apparently semi-cosmetic (only people watching their builds might
wonder), it's fine to fix it whenever.  I'm also moving it to "later," if anyone
objects, feel free to put it back.  :)

Comment 11

[Boris Zbarsky [:bzbarsky]]

Um... reopening.  Otherwise it'll never show up on my radar.  :)  The LATER and
REMIND resolutions are no longer used....

Comment 12

[Boris Zbarsky [:bzbarsky]]

1.1alpha is frozen.  Unsetting milestone and will retriage in a few days when I
can make a realistic assessment of the situation.

Comment 13

[Boris Zbarsky [:bzbarsky]]

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

Comment 14

[David G King]

Just came across this bug, and after reading the coments, I went back to the
code and noticed that there is a #ifdef MOZ_SVG already in
nsExternalHelperAppService.cpp (line 121), although it's in the middle of the
declaration. Could the others be fixed in the same way? Or am I being too hopefull?

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

also does small unrelated changes that I had in my tree

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

-> me

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 129952 [details] [diff] [review]
patch

Make that macro be MAC_TYPE instead of MAC, and r=me.

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 129952 [details] [diff] [review]
patch

bz: ok, I'll make that change

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <-- 
nsExternalHelperAppService.cpp
new revision: 1.213; previous revision: 1.212
done