Last Comment Bug 525463 - AIX Port Seamonkey 2.0: macro redefinition in mailnews/base/src/nsMsgAccountManager.cpp
: AIX Port Seamonkey 2.0: macro redefinition in mailnews/base/src/nsMsgAccountM...
Status: RESOLVED FIXED
: fixed-seamonkey2.0.4
Product: MailNews Core
Classification: Components
Component: Account Manager (show other bugs)
: Trunk
: PowerPC AIX
: -- normal (vote)
: Thunderbird 3.1b2
Assigned To: Uli Link (:ul-mcamafia)
:
:
Mentors:
Depends on: 537967
Blocks: 407295 467015 537588
  Show dependency treegraph
 
Reported: 2009-10-30 08:48 PDT by Uli Link (:ul-mcamafia)
Modified: 2010-03-18 06:40 PDT (History)
5 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
.4-fixed


Attachments
Avoids macro redifinition (1.49 KB, patch)
2010-01-03 05:04 PST, Uli Link (:ul-mcamafia)
mozilla: review-
Details | Diff | Splinter Review
avoids macro redefinition w/o including nsCRT.h (952 bytes, patch)
2010-02-07 15:07 PST, Uli Link (:ul-mcamafia)
no flags Details | Diff | Splinter Review
removing duplicate definitions from nsCRTGlue.h [Checkin: Comment 24] (1.09 KB, patch)
2010-03-07 13:02 PST, Uli Link (:ul-mcamafia)
mozilla: review+
mozilla: superreview+
standard8: approval‑thunderbird3.0.4+
Details | Diff | Splinter Review

Description Uli Link (:ul-mcamafia) 2009-10-30 08:48:02 PDT
User-Agent:       Mozilla/5.0 (X11; U; AIX 5.1; en-US; rv:1.8.1.23) Gecko/20091019 SeaMonkey/1.1.18
Build Identifier: Mozilla/5.0 (X11; U; AIX 5.1; en-US; rv:1.8.1.23) Gecko/20091019 SeaMonkey/1.1.18

xlC_r -o nsMsgAccountManager.o -c  -DMOZ_LDAP_XPCOM -DMOZILLA_INTERNAL_API -DMOZ
_SUITE=1 -DOSTYPE=\"AIX5.1\" -DOSARCH=AIX -DHAVE_MOVEMAIL  -I/home/ulink/Src/com
m-1.9.1/mailnews/base/src -I. -I../../../mozilla/dist/include/xpcom -I../../../m
ozilla/dist/include/alerts -I../../../mozilla/dist/include/string -I../../../moz
illa/dist/include/necko -I../../../mozilla/dist/include/dom -I../../../mozilla/d
ist/include/appshell -I../../../mozilla/dist/include/toolkitcomps -I../../../moz
illa/dist/include/appcomps -I../../../mozilla/dist/include/uconv -I../../../mozi
lla/dist/include/intl -I../../../mozilla/dist/include/htmlparser -I../../../mozi
lla/dist/include/widget -I../../../mozilla/dist/include/docshell -I../../../mozi
lla/dist/include/rdf -I../../../mozilla/dist/include/gfx -I../../../mozilla/dist
/include/thebes -I../../../mozilla/dist/include/layout -I../../../mozilla/dist/i
nclude/content -I../../../mozilla/dist/include/mailnews -I../../../mozilla/dist/
include/locale -I../../../mozilla/dist/include/unicharutil -I../../../mozilla/di
st/include/msgbaseutil -I../../../mozilla/dist/include/webshell -I../../../mozil
la/dist/include/txmgr -I../../../mozilla/dist/include/msgcompose -I../../../mozi
lla/dist/include/msgdb -I../../../mozilla/dist/include/uriloader -I../../../mozi
lla/dist/include/pref -I../../../mozilla/dist/include/msglocal -I../../../mozill
a/dist/include/msgimap -I../../../mozilla/dist/include/mork -I../../../mozilla/d
ist/include/msgnews -I../../../mozilla/dist/include/addrbook -I../../../mozilla/
dist/include/mime -I../../../mozilla/dist/include/mimetype -I../../../mozilla/di
st/include/windowwatcher -I../../../mozilla/dist/include/webbrwsr -I../../../moz
illa/dist/include/exthandler -I../../../mozilla/dist/include/xulapp -I../../../m
ozilla/dist/include/caps -I../../../mozilla/dist/include/xpconnect -I../../../mo
zilla/dist/include/js -I../../../mozilla/dist/include/mozldap -I../../../mozilla
/dist/include   -I../../../mozilla/dist/include/msgbase `/home/ulink/Src/AIX-51/
sm2/mozilla/dist/bin/nspr-config --prefix=/home/ulink/Src/AIX-51/sm2/mozilla/dis
t --includedir=/home/ulink/Src/AIX-51/sm2/mozilla/dist/include/nspr --cflags`   
   -qflag=w:w      -DNDEBUG -DTRIMMED -O2 -qmaxmem=16384 -qalias=noansi   -DMOZI
LLA_1_9_1_BRANCH=1 -DMOZILLA_VERSION=\"1.9.1.4\" -DMOZILLA_VERSION_U=1.9.1.4 -DA
IX=1 -DHAVE_SYS_INTTYPES_H=1 -DNSCAP_DISABLE_DEBUG_PTR_TYPES=1 -DD_INO=d_ino -DS
TDC_HEADERS=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_SIGINFO_T=1 -DHAVE_INT16_T=1 -DHAVE_INT
32_T=1 -DHAVE_INT64_T=1 -DHAVE_INT64=1 -DHAVE_UINT=1 -DHAVE_UINT_T=1 -DHAVE_UINT
16_T=1 -DHAVE_DIRENT_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_NL_TYPES_H=1
 -DHAVE_MALLOC_H=1 -DHAVE_X11_XKBLIB_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_STATF
S_H=1 -DHAVE_LIBC_R=1 -DHAVE_LIBM=1 -DHAVE_LIBDL=1 -DHAVE_LIBC_R=1 -DFUNCPROTO=1
5 -DHAVE_XSHM=1 -D_REENTRANT=1 -DHAVE_RANDOM=1 -DHAVE_STRERROR=1 -DHAVE_LCHOWN=1
 -DHAVE_FCHMOD=1 -DHAVE_SNPRINTF=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DHAVE_STAT64=
1 -DHAVE_LSTAT64=1 -DHAVE_TRUNCATE64=1 -DHAVE_FLOCKFILE=1 -DHAVE_LOCALTIME_R=1 -
DHAVE_STRTOK_R=1 -DHAVE_RES_NINIT=1 -DHAVE_LANGINFO_CODESET=1 -DHAVE_I18N_LC_MES
SAGES=1 -DMOZ_SUITE=1 -DMOZ_BUILD_APP=suite -DMOZ_X11=1 -DMOZ_WIDGET_GTK2=1 -DMO
Z_DISTRIBUTION_ID=\"org.mozilla\" -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DMOZ_
XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_NO_XPCOM
_OBSOLETE=1 -DMOZ_XTF=1 -DMOZ_UPDATE_CHANNEL=default -DMOZ_PLACES=1 -DMOZ_FEEDS=
1 -DMOZ_STORAGE=1 -DMOZ_HELP_VIEWER=1 -DMOZ_LOGGING=1 -DMOZ_USER_DIR=\".mozilla\
" -DMOZ_STATIC_MAIL_BUILD=1 -DHAVE_INTTYPES_H=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING
=1 -DMOZ_RDF=1 -DMOZ_MORK=1 -DMOZ_MORKREADER=1 -DMOZ_DLL_SUFFIX=\".so\" -DXP_UNI
X=1 -DUNIX_ASYNC_DNS=1  -D_COMM_CONFIG_H_ -DMOZILLA_CLIENT /home/ulink/Src/comm-
1.9.1/mailnews/base/src/nsMsgAccountManager.cpp
"../../../mozilla/dist/include/xpcom/nsCRT.h", line 295.9: 1540-0848 (S) The mac
ro name "FILE_ILLEGAL_CHARACTERS" is already defined with a different definition
.
"../../../mozilla/dist/include/msgbase/msgCore.h", line 231.11: 1540-0425 (I) "F
ILE_ILLEGAL_CHARACTERS" is defined on line 231 of "../../../mozilla/dist/include
/msgbase/msgCore.h".
gmake[4]: *** [nsMsgAccountManager.o] Error 1
gmake[4]: Leaving directory `/home/ulink/Src/AIX-51/sm2/mailnews/base/src'
gmake[3]: *** [src_libs] Error 2
gmake[3]: Leaving directory `/home/ulink/Src/AIX-51/sm2/mailnews/base'
gmake[2]: *** [libs_tier_app] Error 2
gmake[2]: Leaving directory `/home/ulink/Src/AIX-51/sm2'
gmake[1]: *** [tier_app] Error 2
gmake[1]: Leaving directory `/home/ulink/Src/AIX-51/sm2'
gmake: *** [default] Error 2


Reproducible: Always

Steps to Reproduce:
1. Compile SEAMONKEY 2.0 on AIX using IBM compiler

Actual Results:  
Build broken

Expected Results:  
Successfull compilation
Comment 1 Uli Link (:ul-mcamafia) 2009-10-30 09:42:31 PDT
The macros FILE_ILLEGAL_CHARACTERS and FILE_PATH_SEPARATOR are defined in mozilla/xpcom/ds/nsCRT.h

the local header msgCore.h is included first, so the the macros aren't defined for the preprocessor yet, so clashing later.

Solution 1: #include "nsCRT.h" after #include "nsCRTGlue.h" in msgCore.h and remove the redundant macro definitions.
nsCRT.h will be added in all units using msgCore.h then

Solution 2: move the #include "msgCore.h" in nsMsgAccountManager.cpp to the end of the header includes so that the #ifndef FILE_PATH_SEPARATOR will do what it is intended for. Affects only the units nsMsgAccountManager.cpp and nsMsgFilterService.cpp
Comment 2 Uli Link (:ul-mcamafia) 2010-01-03 04:59:50 PST
Also with Thunderbird 3.0 from release tarball
Comment 3 Uli Link (:ul-mcamafia) 2010-01-03 05:04:08 PST
Created attachment 419819 [details] [diff] [review]
Avoids macro redifinition

This resolves compilation error on AIX and should not alter the semantics at all.
Comment 4 David :Bienvenu 2010-01-04 14:35:58 PST
Comment on attachment 419819 [details] [diff] [review]
Avoids macro redifinition

If you include "nsCRT.h" as you have, will the code inside the #ifndef's ever be compiled? I guess we're preparing for building with frozen linkages...

nsCRT.h uses the define  OS_FILE_ILLEGAL_CHARACTERS,in the way we're using FILE_ILLEGAL_CHARACTERS, which means we may have been  allowing control characters in file names prior to your patch, possibly, but won't with your patch - that seems like a semantic change, though probably in a good way.

I'm tempted to suggest that we just include "nsCRT.h" and remove the override #defines, since they're a bit broken, as happens when you start cloning code.
Comment 5 David :Bienvenu 2010-01-04 14:36:52 PST
cc'ing Prasad in case he has more insight into this...
Comment 6 Prasad Sunkari [:prasad] 2010-01-05 08:51:54 PST
(In reply to comment #4)
> (From update of attachment 419819 [details] [diff] [review])
> If you include "nsCRT.h" as you have, will the code inside the #ifndef's ever
> be compiled? I guess we're preparing for building with frozen linkages...

As we prepare for the frozen linkages, we may want to avoid including nsCRT.h

> 
> nsCRT.h uses the define  OS_FILE_ILLEGAL_CHARACTERS,in the way we're using
> FILE_ILLEGAL_CHARACTERS, which means we may have been  allowing control
> characters in file names prior to your patch, possibly, but won't with your
> patch - that seems like a semantic change, though probably in a good way.
> 
> I'm tempted to suggest that we just include "nsCRT.h" and remove the override
> #defines, since they're a bit broken, as happens when you start cloning code.

Since "nsCRT.h" anyway includes "nsCRTGlue.h", we should probably consider moving these declarations to "nsCRTGlue.h".  That way users of either headers would be able to use these declarations.
Comment 7 Uli Link (:ul-mcamafia) 2010-01-05 10:24:01 PST
(In reply to comment #6)
> 
> Since "nsCRT.h" anyway includes "nsCRTGlue.h", we should probably consider
> moving these declarations to "nsCRTGlue.h".  That way users of either headers
> would be able to use these declarations.

I suggest creating a new bug in XPCOM for moving these declarations from nsCRT.h to nsCRTGlue.h and this bug blocked by the new bug.
When the new bug is fixed or in advance, I will rework the patch to 
 - not include nsCRT.h (as without my patch)
 - remove the duplicated code

Sometimes it's good to use a very strict compiler, which stops at redefinitions.
But I cannot build/test on trunk, because trunk does not like the IBM compiler ;-)
I'm happy to build 1.9.1 branch now and can surely provide the branch patches.
And it's time to get 1.9.2 branch build on AIX.
Comment 8 Uli Link (:ul-mcamafia) 2010-01-05 11:53:13 PST
For not forgetting:
https://bugzilla.mozilla.org/show_bug.cgi?id=471760#c1
Comment 9 David :Bienvenu 2010-01-06 08:25:53 PST
Comment on attachment 419819 [details] [diff] [review]
Avoids macro redifinition

minusing based on new plan...
Comment 10 Uli Link (:ul-mcamafia) 2010-02-07 15:07:57 PST
Created attachment 425726 [details] [diff] [review]
avoids macro redefinition w/o including nsCRT.h
Comment 11 David :Bienvenu 2010-02-08 11:59:28 PST
Does this really help? msgCore.h is included before nsCRT.h, and nsCRT.h doesn't have similar #ifdef protection, so I still get a bunch of warnings about redefining the illegal chars.
Comment 12 Uli Link (:ul-mcamafia) 2010-02-08 12:21:08 PST
https://bugzilla.mozilla.org/attachment.cgi?id=425726 needs the patch from 
https://bugzilla.mozilla.org/show_bug.cgi?id=537967

Two bugs because spanning different products/modules.
Comment 13 neil@parkwaycc.co.uk 2010-02-15 16:12:31 PST
Does the compiler complain if the redeclaration is for the identical string? We could easily update the macro to match nsCRT.h for now.
Comment 14 Uli Link (:ul-mcamafia) 2010-02-15 17:11:11 PST
No macro redefinition possible without #undef before.
Even with exactly same strings.
Comment 15 neil@parkwaycc.co.uk 2010-03-07 07:44:03 PST
I guess we should now look into removing the definitions from msgCore.h (at least for those branches on which bug 537967 lands).
Comment 16 Uli Link (:ul-mcamafia) 2010-03-07 07:48:49 PST
(In reply to comment #15)
> I guess we should now look into removing the definitions from msgCore.h (at
> least for those branches on which bug 537967 lands).

It has landed on m-c and I will request branch approval also for 1.9.1 and since TB 3.1 will use 1.9.2 also there.
Comment 17 Serge Gautherie (:sgautherie) 2010-03-07 12:16:13 PST
(In reply to comment #11)
> Does this really help?

I wouldn't think so.

(In reply to comment #15)
> I guess we should now look into removing the definitions from msgCore.h (at
> least for those branches on which bug 537967 lands).

Indeed. In fact, this bug is a duplicate of bug 467015, isn't it?

Uli, could you fix it that way, either here or in that older bug?
Comment 18 Uli Link (:ul-mcamafia) 2010-03-07 12:19:34 PST
I suggest removing all the macros defined in nsCRTGlue.h from msgCore.h,
will create a new patch later.
Comment 19 Uli Link (:ul-mcamafia) 2010-03-07 13:02:02 PST
Created attachment 430996 [details] [diff] [review]
removing duplicate definitions from nsCRTGlue.h
[Checkin: Comment 24]
Comment 20 Serge Gautherie (:sgautherie) 2010-03-07 15:01:34 PST
Comment on attachment 430996 [details] [diff] [review]
removing duplicate definitions from nsCRTGlue.h
[Checkin: Comment 24]


Yes, that should be what we want :-)
Comment 21 Uli Link (:ul-mcamafia) 2010-03-08 12:01:17 PST
The nsCRTGlue.h patch has landed on all needed branches.

https://hg.mozilla.org/releases/mozilla-1.9.1/rev/2e523d1e398b
https://hg.mozilla.org/releases/mozilla-1.9.2/rev/03ebb7b90df8
Comment 22 neil@parkwaycc.co.uk 2010-03-09 01:38:14 PST
Comment on attachment 430996 [details] [diff] [review]
removing duplicate definitions from nsCRTGlue.h
[Checkin: Comment 24]

I assume you want this on the comm-1.9.1 branch too.
Comment 23 Uli Link (:ul-mcamafia) 2010-03-09 01:41:14 PST
(In reply to comment #22)
> (From update of attachment 430996 [details] [diff] [review])
> I assume you want this on the comm-1.9.1 branch too.

Yes plz and thx :-)
Comment 24 Serge Gautherie (:sgautherie) 2010-03-09 08:54:56 PST
Comment on attachment 430996 [details] [diff] [review]
removing duplicate definitions from nsCRTGlue.h
[Checkin: Comment 24]


http://hg.mozilla.org/comm-central/rev/bbb5625de160
Comment 25 Mark Banner (:standard8, limited time in Dec) 2010-03-11 05:55:01 PST
Comment on attachment 430996 [details] [diff] [review]
removing duplicate definitions from nsCRTGlue.h
[Checkin: Comment 24]

a=Standard8.

However, please leave the landing to me as I need to co-ordinate picking up latest gecko on our TB 3.0 tinderboxes (some of which are currently tracking the relbranch).
Comment 26 Mark Banner (:standard8, limited time in Dec) 2010-03-16 12:38:26 PDT
Checked into 1.9.1: http://hg.mozilla.org/releases/comm-1.9.1/rev/05a3f1a48963
Comment 27 Mark Banner (:standard8, limited time in Dec) 2010-03-17 09:53:03 PDT
Verified fixed on 3.0 as it hasn't broken the builds, also visually verified what landed in m-1.9.1 was the same as what was removed from comm-1.9.1

Note You need to log in before you can comment on or make changes to this bug.