391141 - trace-malloc doesn't compile on VC8

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

trace-malloc doesn't compile on VC8 because one place in the Windows-specific code depends on incorrect C++ for-loop scoping.  This patch fixes it.

I'd appreciate just a sanity check that this is correct; approval at the same time would be appreciated.  (I suppose I could squeeze a PR_ASSERT into the for loop's condition with the , operator, but that seems even uglier.)

Comment 1

[Brendan Eich [:brendan]]

Comment on attachment 275482 [details] [diff] [review]
patch

Even better would be to rewrite using double indirection to unify the basis and inductive cases.

/be

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk, 2007-08-10 14:26 -0700; I'm not going to worry about rewriting, although it is pretty simple...

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: address review comment

This addresses comment 1.  I've had it in my tree for a while (although I just fixed a mistake in what I had after testing it for the first time).

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 281565 [details] [diff] [review]
address review comment

ok by me, but the comma expression as condition is a bit hard on the eyes. But fixing that seems to require a while loop. Alternative: lose the non-null assert, since a null pointer deref is just as good in my book as an assertion (clean, unexploitable crash pointing to the offending code in this case, or as close to the offending code as the PR_ASSERT in this patch points).

/be

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Above patch checked in to trunk.

Comment 6

[[github robot]]

Commit pushed to master at https://github.com/mozilla/webmaker.org

https://github.com/mozilla/webmaker.org/commit/e8d3bbb7cdc3a377e4079b4cb9c44ef1f47ba3ae
Bug 391141 - Added appmaker to tools menu

Comment 7

[inactivate account for kate]

Sorry wrong bug number