On Jun 8, 2012, at 10:07 AM, Johan Engelen wrote:
154 g_return_if_fail (cl != NULL); 155 g_return_if_fail (SP_IS_CTRLLINE (cl));" and recommends we drop these two lines. There is a subtle change going from sp_classname_set(class*) to class->set(). From a callers perspective, the former seems to accept NULL, whereas the latter does not. If the caller relied on NULL pointer handling, at least the first line should be kept, or the calling code should be adjusted. There have been a number of crashers that involved class methods being called with this=NULL. It is easy to recognize such crashes, and a fix is easy. I agree we can remove those lines, but not when release is getting close...!
Well... the first point is that g_return_if_fail() not only returns, but issues a g_warning() first. No code should be using null pointers in general, and *if* code we have did feed one to one of these functions, then we'd be seeing g_warnings() all over the place.
Of course, a better check would be to run things under valgrind and hit the scenarios likely to cause problems. Then the best for finding hidden use of NULL pointers would be to check Coverity results.
*However* the main change would be in making such functions methods instead of stand-alone functions. In those cases a null pointer will cause things to blow up before we get a chance to check it. So we need to get all our code safely not feeding null pointers around anyway.
Question: For me a big benefit of C++ classes is virtual methods (notably virtual destructors!). Any way we can get rid of the very ugly and bugprone parent_class stuff?
There are a few ways. One that looks promising is to have a FooImpl friend class as a private member of a Foo class that is a GObject subclass (aka created and maintained by GTK and using the parent-class paradigm). Then in the Foo init function the new FooImpl would be created, and then released also when appropriate.
If we use a pointer for this, then the FooImpl class remains hidden within foo.cpp and we can change it at will without even causing more than a single file to recompile. Or if we want to use some public C++ class as a member we can use placement new.
Of course, since we have C++ itself to create an manage instances, we could move more code away from using GTK+/Gobject to just plain C++.