Closed Bug 795507 Opened 12 years ago Closed 12 years ago

Remove the usages of PR_BEGIN_EXTERN_C and PR_END_EXTERN_C from the tree


(Core :: General, defect)

Not set





(Reporter: ehsan.akhgari, Assigned: isaac)


(Blocks 1 open bug)


(Whiteboard: [mentor=ehsan][lang=c++])


(2 files, 1 obsolete file)

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

#ifdef __cplusplus
extern "C" {

// ... some code

#ifdef __cplusplus

Inside .cpp files, the #ifdef dance above is not needed, because, well, we'll be compiling in C++ mode.  :-)
Assignee: nobody → isaac.aggrey
Try run for 5d7ef44fa6da is complete.
Detailed breakdown of the results available here:
Results (out of 16 total builds):
    exception: 16
Builds (or logs if builds failed) available at:
Attachment #666387 - Flags: review?(ehsan)
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
Try run for 60ff6f4ebdb5 is complete.
Detailed breakdown of the results available here:
Results (out of 66 total builds):
    exception: 25
    success: 32
    failure: 9
Builds (or logs if builds failed) available at:
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.  :-)
Attachment #666387 - Flags: review?(ehsan)
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.
Attachment #666387 - Attachment is obsolete: true
Attachment #666419 - Flags: review?(ehsan)
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!
Attachment #666419 - Flags: review?(ehsan) → review+
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
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).
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).
Resolution: FIXED → ---
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.  :(
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.
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
Sure, did that in bug 799717.
(In reply to Ehsan Akhgari [:ehsan] from comment #14)
> Backout:

Merge of backout:
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.