402395 - Dangling semi-colon caused when "ac_add_options --disable-logging" throws warning in VS2005

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Michael Mullin]

Attachment: quick hack to get the build going

Trunk build on windows vs2005 breaks when "ac_add_options --disable-logging" is used in a mozconfig

When PR_LOGGING is not defined, the COOKIE_LOGSTRING macro is mapped to nothing (line 328); thus line 437 has a dangling semi-colon when PR_LOGGING is not defined.  VS2005 throws a warning for dangling semi-colons, and the build treats warnings as errors.

Comment 1

[Michael Mullin]

Attachment: More long term patch

Comment 2

[Michael Mullin]

Attachment: Real patch (previous one had bug)

Comment 3

[Jesse Ruderman]

Comment on attachment 287281 [details] [diff] [review]
Real patch (previous one had bug)

This patch fixes a problem similar to the one fixed in bug 260209.  The patch in that bug used the "do{}while(0)" trick, whereas this patch uses a simpler trick.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Why don't you use PR_BEGIN_MACRO and PR_END_MACRO like my patch in bug 402506?

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: proposal patch

proposal patch, using PR_BEGIN_MACRO and PR_END_MACRO.

Comment 7

[Michael Mullin]

I think that "0;" is about as simple as you can get to solve this problem, and I tried to be simple.

However, it seems like the "PR_BEGIN_MACRO /* nothing */ PR_END_MACRO" is the norm to solve this problem in the moz-code.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #5)
> Why don't you use PR_BEGIN_MACRO and PR_END_MACRO like my patch in bug 402506?

Why is that better than using 0? Using 0 seems simpler.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

PR_BEGIN_MACRO/PR_END_MACRO hack is already used, so, it is safety on all our supporting platforms. If "0;" is better, we should use it on other similar points.

Comment 10

[Benjamin Smedberg]

Comment on attachment 287281 [details] [diff] [review]
Real patch (previous one had bug)

The other one is better

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

bsmedberg recommends that we don't block beta on this, moving TM out to
"undefined", I'll let the component owner make the final blocking call.

(blocking? w/TM=M9 is asking for beta-blocking status)

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 287377 [details] [diff] [review]
proposal patch

a=beltzner for M9

Comment 13

[[e-mail]]

checked in for M9. thanks for the patches!