On Mon, Jul 22, 2013 at 12:46 PM, Benjamin Smedberg
<benja...@smedbergs.us>wrote:
> On 7/22/2013 3:01 PM, Rob Arnold wrote:
>
>>
>> Wouldn't disallowing this implicit conversion break code which does
>>
>> void UseFoo(nsIFoo* foo);
>>
>> nsCOMPtr<nsIFoo> foo;
>> UseFoo(foo);
>>
>> ? That is an extremely common idiom in our code.
>>
>> That idiom seems dangerous by itself; the assumption it makes is that the
>> reference will outlive the callee but that isn't actually enforced
>> anywhere
>>
> Doesn't the nsCOMPtr<nsIFoo> foo; enforce it? I'm confused by what you're
> arguing.
In this case, yes. I'm saying that there are others where this cannot be
enforced (see below).
> (you could write UseFoo(nsRefPtr<T>(new T)) and Bad Things would happen;
>>
> What would be bad about it? The T stays alive during the call to UseFoo.
Ah, you're right. Bad example. 'UseFoo(new T)' compiles but leaks,
'nsRefPtr<T> foo() { nsRefPtr<T> bar(...); ...; return bar; }' is a
use-after-tree in cases like 'T* p = foo();' and there are probably a
number of other cases that the compiler allows and/or writes for you where
issues can arise.
The end goal that I am promoting is to never deal with raw pointers because
the ownership semantics aren't clear; instead, shove the pointer into a
strong container like RefPtr or std::unique_ptr and let the strong
containers/compiler handle the safety. I know that with earlier
compilers/editions of C++, it was not possible to do this efficiently, but
I think it is worth revisiting this in a discussion of modernizing the
codebase with C++11. I believe that it's mozilla style to never return an
nsRefPtr from a function/method but with C++11 move semantics and
optimizations like RVO, you can do this efficiently in a number of common
cases.
> there are less obvious ways to write this). I would rewrite UseFoo to take
>> an const nsRefPtr<nsIFoo>& instead
>>
> Except that you'd basically be rewriting our entire tree, including all of
> XPIDL. Let's not try to boil that ocean for marginal benefit.
>
I haven't been very involved in Mozilla development for a while but my
understanding based on when I stopped contributing code is that a) we've
done large code modifications before, b) our static analysis tools make it
easier to maintain these and c) I am optimistic that these changes can be
done incrementally if need be. Thus, I am a bit skeptical that this change
requires an ocean boil, but I am not the one doing the work so really it's
just an armchair opinion (I did a similar migration on a 30kloc codebase
without significant issues and Mozilla has better test coverage). I think
improved memory safety and lower cognitive overhead for reasoning about the
safety of the code is an important, not marginal, benefit for both security
vulnerabilities and the project's hackability (in the code contribution
sense).
-Rob
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
