Closed Bug 1309111 Opened 3 years ago Closed 3 years ago

Some macros are not expanded correctly by MSVC

Categories

(Core :: Audio/Video: Playback, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(2 files)

For
  
  #define FMT(x, ...) "this=%p " x, this, ##__VA_ARGS__
  #define LOG(...) printf(FMT(__VA_ARGS__))

LOG("foo=%d", 47) is expanded by MSVC to

  printf("this=%p foo=%d", 47, this );

Changing LOG to

  #define LOG(x, ...) printf(FMT(x, ##__VA_ARGS__))

will fix the problem.
Assignee: nobody → jwwang
Priority: -- → P3
Attachment #8800488 - Flags: review?(gsquelart)
Comment on attachment 8800488 [details]
Bug 1309111 - Some macros are not expanded correctly by MSVC.

https://reviewboard.mozilla.org/r/85402/#review83980
Attachment #8800488 - Flags: review?(gsquelart) → review+
Thanks!
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b0aefa7aa503
Some macros are not expanded correctly by MSVC. r=gerald
Btw, this is responsible for Windows crashes when mochitests time out and try to call MediaDecoderStateMachine::DumpDebugInfo().
https://hg.mozilla.org/mozilla-central/rev/b0aefa7aa503
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Duplicate of this bug: 1329057
Comment on attachment 8824341 [details] [diff] [review]
1309111_fix_beta_51.patch

Approval Request Comment
[Feature/Bug causing the regression]:1309111
[User impact if declined]:Mochitests will crash if timeout happens on Windows.
[Is this code covered by automated tests?]:Yes
[Has the fix been verified in Nightly?]:Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]:None
[Is the change risky?]:No
[Why is the change risky/not risky?]:The fix is simple and has been tested on 52 and 53.
[String changes made/needed]:none
Attachment #8824341 - Flags: approval-mozilla-beta?
Comment on attachment 8824341 [details] [diff] [review]
1309111_fix_beta_51.patch

Sounds like a useful test fix, let's uplift it to beta 51.
Attachment #8824341 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1330972
You need to log in before you can comment on or make changes to this bug.