Does it? I don't think I'm any more or less likely to omit a validity check using operator->() vs get(). Maybe it's just me.
It seems like get() might actually be *more* prone to failure. Imagine: Class* object = foo.get(); if (object) { object->Method(); } // ... A lot of stuff happens and the ref blows up ... if (object) { object->Method(); // oops } ----- Original Message ----- From: "Ehsan Akhgari" <ehsan.akhg...@gmail.com> To: "David Major" <dma...@mozilla.com> Cc: dev-platform@lists.mozilla.org Sent: Tuesday, October 8, 2013 4:27:03 PM Subject: Re: Audit your code if you use mozilla::WeakPtr On 2013-10-08 7:10 PM, David Major wrote: > Isn't it ultimately up to the developer to get it right? Someone could just > as well forget to use |if (object)| from your example. > > Here's a sample usage from the header file: > * // Test a weak pointer for validity before using it. > * if (weak) { > * weak->num = 17; > * weak->act(); > * } Sure, but that "convenience" operator makes it natural to write incorrect code by default. It's better to be explicit and correct, than implicit and wrong. :-) Cheers, Ehsan > ----- Original Message ----- > From: "Ehsan Akhgari" <ehsan.akhg...@gmail.com> > To: dev-platform@lists.mozilla.org > Sent: Tuesday, October 8, 2013 3:54:17 PM > Subject: Audit your code if you use mozilla::WeakPtr > > I and Benoit Jacob discovered a bug in WeakPtr (bug 924658) which makes its > usage unsafe by default. The issue is that WeakPtr provides convenience > methods such as operator->, which mean that the consumer can directly > dereference it without the required null checking first. This means that > you can have code like the below: > > WeakPtr<Class> foo = realObject->asWeakPtr(); > // ... > foo->Method(); > > That will happily compile and will crash at runtime if the object behind > the weak pointer is dead. The correct way of writing such code is: > > Class* object = foo.get(); > if (object) { > object->Method(); > } > > I don't know enough about all of the places which use WeakPtr myself to fix > them all, but if you have code using this in your module, please spend some > time auditing the code, and fix it. Please file individual bugs for your > components blocking bug 924658. > > Thanks! > -- > Ehsan > <http://ehsanakhgari.org/> > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform