On Tuesday, August 12, 2014 11:22:05 AM UTC-4, Aryeh Gregor wrote:
> For refcounted types, isn't a raw pointer in a local variable a red
> flag to reviewers to begin with? If GetT() returns a raw pointer
> today, like nsINode::GetFirstChild() or something, storing the result
> in a raw pointer is a potential use-after-free, and that definitely
> has happened already. Reviewers need to make sure that refcounted
> types aren't ever kept in raw pointers in local variables, unless
> perhaps it's very clear from the code that nothing can possibly call
> Release() (although it still makes me nervous).
Putting the burden on reviewers when something can be automatically checked
doesn't seem like a good idea -- it requires reviewers to know that GetT()
*does* return a refcounted type, for example. As dbaron pointed out, there are
cases where we do actually return and keep things around as bare pointers.
It's unfortunate that we can't create a nsCOMPtr<> that will disallow
assignment to a bare pointer without an explicit .get(), but will still allow
conversion to a bare pointer for arg passing purposes. (Or can we? I admit my
C++-fu is not that strong in this area...) It would definitely be nice to get
rid of already_AddRefed<> (not least because the spelling of "Refed" always
grates when I see it :).
- Vlad
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
