Certainly I would rather work towards committing to the main repository, but I realize that usually takes some time to establish trust that I won't screw up the codebase.
Realizing that although I had commit rights I would continue to submit patches until the maintainer said it was OK to start making use of my commit rights, I started off by submitting this simple patch for something I noticed while debugging an unrelated issue:
--- dvbtab.cpp (revision 856017)
+++ dvbtab.cpp (working copy)
@@ -110,7 +110,8 @@
DvbTab::~DvbTab()
{
- delete dvbStream;
+ if (dvbStream != NULL)
+ delete dvbStream;
}
void DvbTab::configureChannels()
@@ -142,8 +143,10 @@
manager->getMediaWidget()->stop();
- delete dvbStream;
- dvbStream = NULL;
+ if (dvbStream != NULL) {
+ delete dvbStream;
+ dvbStream = NULL;
+ }
if ((liveDevice == NULL) || (liveChannel->source != channel->source)) {
if (liveDevice != NULL) {
Pretty straightfoward? I'm checking that dvbStream isn't NULL before doing the delete. Except, as the maintainer politely pointed out, this is perfectly valid according to the C++ standard. In fact, it's even in the C++ FAQ:
From http://www.faqs.org/faqs/C++-faq/part6/
[16.7] Do I need to check for NULL before delete p?
No!
The C++ language guarantees that delete p will do nothing if p is equal to
NULL. Since you might get the test backwards, and since most testing
methodologies force you to explicitly test every branch point, you should not
put in the redundant if test.
Wrong:
if (p != NULL)
delete p;
Right:
delete p;
Wow. Do I feel like a moron. I knew that gcc wouldn't have an issue if you fed NULL to delete, but I had assumed it was a gcc'ism and other compilers might choke.
Now I consider myself "reasonably technical" when it comes to C/C++, since it's been my full time job for the last ten years. I've contributed to around a dozen different open source projects, wrote a couple of hundred thousand lines of code, and yet I made this rookie mistake and managed to make myself look like an idiot within an hour of being granted svn commit rights to a project.
I hang my head in shame.
Note to self: When I get home tonight, read the C++ FAQ. There looks like there is some good stuff there...
Update: Dan also thought it was illegal to delete NULL. I guess that make me feel a little better.
