In <TestPipe.cpp>, "warning C4273: 'NS_NewPipe2' : inconsistent dll linkage", when without |--enable-libxul|

RESOLVED FIXED in mozilla1.9.1b1

Status

()

Core
XPCOM
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

Trunk
mozilla1.9.1b1
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

9 years ago
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1a2pre) Gecko/2008080519 SeaMonkey/2.0a1pre] (home, optim default) (W2Ksp4)

{{
.../xpcom/tests/TestPipe.cpp(51) : warning C4273: 'NS_NewPipe2' : inconsistent dll linkage
        ...\dist\include\xpcom\nsIPipe.h(290) : see previous definition of 'NS_NewPipe2'

}}
(Assignee)

Comment 1

9 years ago
Created attachment 332419 [details] [diff] [review]
(Av1) <TestPipe.cpp>

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1a2pre) Gecko/2008080519 SeaMonkey/2.0a1pre] (home, optim default) (W2Ksp4)

Remove obsolete duplicated function.

See
<http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsPipe3.cpp#1292>
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #332419 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #332419 - Flags: review? → review?(bsmedberg)
(Assignee)

Updated

9 years ago
Attachment #332419 - Flags: review?(bsmedberg) → review?(benjamin)
(Assignee)

Updated

9 years ago
Blocks: 450163
(Assignee)

Updated

9 years ago
No longer blocks: 450163
(Assignee)

Updated

9 years ago
Attachment #332419 - Flags: superreview?(benjamin)

Comment 2

9 years ago
Comment on attachment 332419 [details] [diff] [review]
(Av1) <TestPipe.cpp>

This will break --enable-tests --enable-libxul, because NS_NewPipe2 is not exported from XPCOM in that case.
Attachment #332419 - Flags: superreview?(benjamin)
Attachment #332419 - Flags: review?(benjamin)
Attachment #332419 - Flags: review-
(Assignee)

Comment 3

9 years ago
Created attachment 333971 [details] [diff] [review]
(Av2) <TestPipe.cpp> ++

(In reply to comment #2)
> (From update of attachment 332419 [details] [diff] [review])
> This will break --enable-tests --enable-libxul, because NS_NewPipe2 is not
> exported from XPCOM in that case.

Confirmed with Firefox non-debug:
{{
TestPipe.obj : error LNK2019: unresolved external symbol "unsigned int __cdecl NS_NewPipe2(class nsIAsyncInputStream * *,class nsIAsyncOutputStream * *,int,int,unsigned int,unsigned int,class nsIMemory *)" (?NS_NewPipe2@@YAIPAPAVnsIAsyncInputStream@@PAPAVnsIAsyncOutputStream@@HHIIPAVnsIMemory@@@Z) referenced in function "unsigned int __cdecl TestBackwardsAllocator(void)" (?TestBackwardsAllocator@@YAIXZ)
TestPipe.exe : fatal error LNK1120: 1 unresolved externals
}}
I was misleaded because SeaMonkey doesn't use |--enable-libxul| (yet).

*****

Av1, with comment 2 suggestion(s).
Compiled with both default and debug Firefox builds.

1) Add |#ifdef| to fix the warning.
2) s/size_t/PRUint32/ to further sync with <nsIPipe.h>.
3) Additional minor code sync/optim in <nsPipe3.cpp>.
Attachment #332419 - Attachment is obsolete: true
Attachment #333971 - Flags: superreview?(benjamin)
Attachment #333971 - Flags: review?(benjamin)
(Assignee)

Updated

9 years ago
Flags: in-testsuite-
Summary: In <TestPipe.cpp>, "warning C4273: 'NS_NewPipe2' : inconsistent dll linkage" → In <TestPipe.cpp>, "warning C4273: 'NS_NewPipe2' : inconsistent dll linkage", when without |--enable-libxul|

Comment 4

9 years ago
Comment on attachment 333971 [details] [diff] [review]
(Av2) <TestPipe.cpp> ++

Why don't you either get rid of the NS_NewPipe2 method in this file altogether (inlining the code) or rename the method so it doesn't conflict?

Please do not include extraneous changes such as the "rv" move at the top of this patch. Declaring nsresult rv at the top of a function is good programming practice in XPCOM.
Attachment #333971 - Flags: superreview?(benjamin)
Attachment #333971 - Flags: review?(benjamin)
Attachment #333971 - Flags: review-
(Assignee)

Comment 5

9 years ago
Created attachment 334064 [details] [diff] [review]
(Av3) <TestPipe.cpp>
[Checkin: Comment 6]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1a2pre) Gecko/20080816024259 SeaMonkey/2.0a1pre] (home, optim default) (W2Ksp4)

Av2, with comment 4 suggestion(s).

1) s/NS_NewPipe2/TP_NewPipe2/ to fix the warning.
2) s/size_t/PRUint32/ to further sync with <nsIPipe.h>.
Attachment #333971 - Attachment is obsolete: true
Attachment #334064 - Flags: superreview?(benjamin)
Attachment #334064 - Flags: review?(benjamin)

Updated

9 years ago
Attachment #334064 - Flags: superreview?(benjamin)
Attachment #334064 - Flags: review?(benjamin)
Attachment #334064 - Flags: review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
(Assignee)

Comment 6

9 years ago
Comment on attachment 334064 [details] [diff] [review]
(Av3) <TestPipe.cpp>
[Checkin: Comment 6]

http://hg.mozilla.org/mozilla-central/rev/b66de969b8c5
Attachment #334064 - Attachment description: (Av3) <TestPipe.cpp> → (Av3) <TestPipe.cpp> [Checkin: Comment 6]
(Assignee)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1a2 → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.