trace-malloc doesn't compile on VC8

RESOLVED FIXED in mozilla1.9alpha8

Status

()

Core
General
RESOLVED FIXED
11 years ago
4 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
mozilla1.9alpha8
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Created attachment 275482 [details] [diff] [review]
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.)
Attachment #275482 - Flags: review?(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
Attachment #275482 - Flags: review?(brendan)
Attachment #275482 - Flags: review+
Checked in to trunk, 2007-08-10 14:26 -0700; I'm not going to worry about rewriting, although it is pretty simple...
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Created attachment 281565 [details] [diff] [review]
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).
Attachment #281565 - Flags: review?(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
Attachment #281565 - Flags: review?(brendan) → review+
Attachment #281565 - Flags: approval1.9?
Above patch checked in to trunk.

Updated

11 years ago
Attachment #281565 - Flags: approval1.9? → approval1.9+
Comment hidden (off-topic)
Comment hidden (off-topic)
You need to log in before you can comment on or make changes to this bug.