795507 - Remove the usages of PR_BEGIN_EXTERN_C and PR_END_EXTERN_C from the tree

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[:ehsan akhgari]

Inside headers, these macros should be replaced by something like this:

#ifdef __cplusplus
extern "C" {
#endif

// ... some code

#ifdef __cplusplus
}
#end

Inside .cpp files, the #ifdef dance above is not needed, because, well, we'll be compiling in C++ mode.  :-)

Comment 1

[Mozilla RelEng Bot]

Try run for 5d7ef44fa6da is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5d7ef44fa6da
Results (out of 16 total builds):
    exception: 16
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-5d7ef44fa6da

Comment 2

[Isaac Aggrey [:isaac]]

Attachment: Script to replace PR_BEGIN_EXTERN_C and PR_END_EXTERN_C

Comment 3

[Isaac Aggrey [:isaac]]

Attachment: Remove PR_BEGIN_EXTERN_C and PR_END_EXTERN_C from tree

Comment 4

[Isaac Aggrey [:isaac]]

Ignore the try run results posted above. I cancelled my earlier try run because I keep prematurely pushing to try before building/testing on my machine to catch silly mistakes; fortunately, I cancel them before they run more than a few .

Try results will be at https://tbpl.mozilla.org/?tree=Try&rev=60ff6f4ebdb5

Comment 5

[Mozilla RelEng Bot]

Try run for 60ff6f4ebdb5 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=60ff6f4ebdb5
Results (out of 66 total builds):
    exception: 25
    success: 32
    failure: 9
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-60ff6f4ebdb5

Comment 6

[:ehsan akhgari]

Comment on attachment 666387 [details] [diff] [review]
Remove PR_BEGIN_EXTERN_C and PR_END_EXTERN_C from tree

Review of attachment 666387 [details] [diff] [review]:
-----------------------------------------------------------------

::: dbm/include/mcom_db.h
@@ +415,5 @@
>  #endif
>  
> +#ifdef __cplusplus
> +}
> +#end

I believe this should be #endif here, as try server results indicate.  :-)

Comment 7

[Isaac Aggrey [:isaac]]

Attachment: Remove PR_BEGIN_EXTERN_C and PR_END_EXTERN_C from tree (fixed)

Oops, I really should be looking at my diffs since I thought I pushed the corrected version. I made the incorrect assumption that my revert in source control and re-application of the script caught everything. Another lesson learned!

Thanks, Ehsan.

Comment 8

[:ehsan akhgari]

Comment on attachment 666419 [details] [diff] [review]
Remove PR_BEGIN_EXTERN_C and PR_END_EXTERN_C from tree (fixed)

Review of attachment 666419 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

Comment 9

[:ehsan akhgari]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d1a365a3320

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2d1a365a3320

Comment 11

[Kai Engert (:kaie:)]

Sorry guys, you overlooked that directory mozilla/dbm is owned by the NSS library, and you must not change it except by upgrading to the new NSS release.

The upstream for any changes to this directory is the NSS project.

(I guess we'll have to make that clearer by adding a file to that directory....?)

How shall we proceed?

Either you try to get review to the changes to file dbm/include/mcom_db.h TODAY and we check it in to upstream NSS, or it should be backed out from mozilla-central.

The reason for the urge is that we're about to create a NSS 3.14 release (by thursday).

Comment 12

[Kai Engert (:kaie:)]

Note that I'll backout the mozilla/dbm portion if nothing happens by tomorrow (it will be backed out / overwritten automatically anyway, as soon as we upgrade to 3.14 final release).

Comment 13

[:ehsan akhgari]

Sigh...  I'm not sure how easy it would be to get reviews in the upstream project.  My experience on the review turn-around time in the past has mostly been weeks, not days.  So I guess I'll backout those parts.  :(

Comment 14

[:ehsan akhgari]

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/830fccc0509c

Comment 15

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Ehsan, there should be no Mozilla C++ code including these DBM headers anyway, so this should not affect bug 796941 anyway. So, while I'd be happy to r+ your patch and check it into the NSS tree, I don't think it is necessary.

Comment 16

[Kai Engert (:kaie:)]

Thank you very much for backing out from mozilla-central!

I propose to simply file a new NSS bug, attach the patch, have bsmith r+ it, and we'll check it in to NSS. thanks

Comment 17

[:ehsan akhgari]

Sure, did that in bug 799717.

Comment 18

[Ed Morley [:emorley]]

(In reply to Ehsan Akhgari [:ehsan] from comment #14)
> Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/830fccc0509c

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/830fccc0509c