The patches in bug 514872 fixed the problem of forgetting to copy the expression, but it does so before checking whether the allocation succeeds. If for some reason it fails (and if it never does why are we checking at all?) then we'll crash on a null deref doing memcpy(0,...) Not likely in practice, and we've talked about making allocation failures instantly fatal anyway, but in the meantime static source checkers like Coverity are going to complain at us.
I'm really appalled at having missed the one thing there possibly was to miss about this fix... Anyway, let's get this done. :-\
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #411161 - Flags: review?(jwalden+bmo)
Attachment #411161 - Flags: review?(jwalden+bmo) → review+
Blah, I dropped the ball on this - sorry! The tree is currently restricted - should we attempt to still get this for 3.6? It's pretty harmless, as far as I can tell... dveditz, what do you reckon?
Comment on attachment 411161 [details] [diff] [review] Patch moving the memcpy below the nullcheck Doesn't block 1.9.2, but is wanted on that branch and safe to take.
We'll want this on the other branches that got 514872, too.
Comment on attachment 411161 [details] [diff] [review] Patch moving the memcpy below the nullcheck a=beltzner for all branches, please land on 1917 and 19017 when the builds are done for 1916 and 19016
(In reply to comment #5) > (From update of attachment 411161 [details] [diff] [review]) > a=beltzner for all branches, please land on 1917 and 19017 when the builds are > done for 1916 and 19016 Gah, missed that train (well, .7beta is out, according to the testing announcement in .planning). From the announcement there of .6 being released (the 16th) there was an 8-day window to check this in - but you wrote "when the builds are done", not when the release is announced - is there a better thing than the release announcement I should be looking for in terms of the .7 builds being Done so I can land this for .8 ? Also, this bug was originally marked blocking 220.127.116.11+, and is now .8+, but there was no comment on this bug about it being pushed back (or asking me to land it soon please because...). I'm not trying to shift blame here, but I'm also not really clear on how this blocking mechanism then works. :-)
.7 and .17 were pushed forward for a few specific fixes, and are being shipped from the previous relbranch - this bug specifically wasn't pushed back. You're free to land on 1.9.0/1.9.1 now for 18.104.22.168/22.214.171.124.
(In reply to comment #8) > .7 and .17 were pushed forward for a few specific fixes, and are being shipped > from the previous relbranch - this bug specifically wasn't pushed back. You're > free to land on 1.9.0/1.9.1 now for 126.96.36.199/188.8.131.52. OK, great, thanks for clarifying! Will push those changes as soon as I can get to it inbetween the goings-on these days... Merry Christmas! :-)
Checking in nsWildCard.cpp; /cvsroot/mozilla/xpfe/components/filepicker/src/nsWildCard.cpp,v <-- nsWildCard.cpp new revision: 1.7; previous revision: 1.6 done http://hg.mozilla.org/releases/mozilla-1.9.1/rev/076725c55412
You need to log in before you can comment on or make changes to this bug.