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.)
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
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.
You need to log in before you can comment on or make changes to this bug.