Closed Bug 525463 Opened 11 years ago Closed 11 years ago

AIX Port Seamonkey 2.0: macro redefinition in mailnews/base/src/nsMsgAccountManager.cpp

Categories

(MailNews Core :: Account Manager, defect)

PowerPC
AIX
defect
Not set
normal

Tracking

(thunderbird3.0 .4-fixed)

RESOLVED FIXED
Thunderbird 3.1b2
Tracking Status
thunderbird3.0 --- .4-fixed

People

(Reporter: ul-mcamafia, Assigned: ul-mcamafia)

References

Details

(Keywords: fixed-seamonkey2.0.4)

Attachments

(1 file, 2 obsolete files)

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
Version: unspecified → Trunk
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
Component: MailNews: Account Configuration → Account Manager
Product: SeaMonkey → MailNews Core
Version: Trunk → 1.9.1 Branch
Also with Thunderbird 3.0 from release tarball
Attached patch Avoids macro redifinition (obsolete) — Splinter Review
This resolves compilation error on AIX and should not alter the semantics at all.
Blocks: 537588
Attachment #419819 - Flags: review?(bienvenu)
QA Contact: mailnews-account → account-manager
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.
cc'ing Prasad in case he has more insight into this...
Status: UNCONFIRMED → NEW
Ever confirmed: true
(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.
(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.
Depends on: 537967
Comment on attachment 419819 [details] [diff] [review]
Avoids macro redifinition

minusing based on new plan...
Attachment #419819 - Flags: review?(bienvenu) → review-
Attachment #419819 - Attachment is obsolete: true
Attachment #425726 - Flags: review?
Attachment #425726 - Flags: review? → review?(bienvenu)
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.
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.
Does the compiler complain if the redeclaration is for the identical string? We could easily update the macro to match nsCRT.h for now.
No macro redefinition possible without #undef before.
Even with exactly same strings.
Assignee: nobody → ul.mcamafia
Status: NEW → ASSIGNED
I guess we should now look into removing the definitions from msgCore.h (at least for those branches on which bug 537967 lands).
(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.
(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?
I suggest removing all the macros defined in nsCRTGlue.h from msgCore.h,
will create a new patch later.
Attachment #425726 - Attachment is obsolete: true
Attachment #425726 - Flags: review?(bienvenu)
Comment on attachment 430996 [details] [diff] [review]
removing duplicate definitions from nsCRTGlue.h
[Checkin: Comment 24]


Yes, that should be what we want :-)
Attachment #430996 - Flags: superreview?(bienvenu)
Attachment #430996 - Flags: review?(bienvenu)
Attachment #430996 - Flags: superreview?(bienvenu)
Attachment #430996 - Flags: superreview+
Attachment #430996 - Flags: review?(bienvenu)
Attachment #430996 - Flags: review+
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.
Attachment #430996 - Flags: approval-thunderbird3.0.4?
(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 :-)
Depends on: 549100
No longer depends on: 549100
Comment on attachment 430996 [details] [diff] [review]
removing duplicate definitions from nsCRTGlue.h
[Checkin: Comment 24]


http://hg.mozilla.org/comm-central/rev/bbb5625de160
Attachment #430996 - Attachment description: removing duplicate definitions from nsCRTGlue.h → removing duplicate definitions from nsCRTGlue.h [Checkin: Comment 24]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1b2
Version: 1.9.1 Branch → Trunk
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).
Attachment #430996 - Flags: approval-thunderbird3.0.4? → approval-thunderbird3.0.4+
Whiteboard: [Standard8 to land post moving TB 3 tinderboxes onto the next relbranch]
Checked into 1.9.1: http://hg.mozilla.org/releases/comm-1.9.1/rev/05a3f1a48963
Whiteboard: [Standard8 to land post moving TB 3 tinderboxes onto the next relbranch]
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
You need to log in before you can comment on or make changes to this bug.