'Twas brillig, and David Henningsson at 09/11/11 09:42 did gyre and gimble:
> So; I think we've discussed the general case enough, back to where we
> started:
>
> I think this is cleaner and gives better error handling:
>
> void foo_free(foo* f)
> {
> if (!f)
> return;
> /* Possibly more destruction stuff here */
> pa_xfree(f);
> }
>
>
> /* Somewhere else */
> foo_free(bar->f);
>
> Than this:
>
> void foo_free(foo* f)
> {
> pa_assert(f);
> /* Possibly more destruction stuff here */
> pa_xfree(f);
> }
>
> /* Somewhere else */
> if (bar->f)
> foo_free(bar->f);
>
>
> The most common case for bar->f being null is that the bar object was
> not completely initialized. In addition, in destructors you should never
> throw exceptions.
>
> If adjusting my code from the IMO better approach to the IMO worse
> approach is what it takes to get the code in, I'll obviously do it. Let
> me know if that is necessary.
I could fairly easily be persuaded to agree that for "free" functions as
you listed above, asserting is likely overkill. After all pa_xfree(NULL)
is quite happy, so why should any higher level free function complain
more bitterly?
But free functions are arguably a particular class of function. Not
calling free properly only (usually) results in memory leaks.
I certainly wouldn't like a module to call pa_sink_input_move_to() with
invalid pointers and have that handled gracefully. I'd like the
conditions that lead up to the incorrect call to be very much reported
in a backtrace should it ever get out in a release.
Otherwise what happens? The stream is not moved and a something gets
written in a log and some key functionality just doesn't work but often
the user will be none the wise... "Hmm, I thought plugging in my USB was
meant to switch my music to it but it didn't... ho hum never mine"...
just an anonymous entry in a log and nothing gets reported etc. etc.
So for this class of problem, that is clear programming error - using
the APIs wrong - I'm still very much in favour of the assert usage.
But that's just my opinion, and I know it's certainly far from universal.
Col
--
Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/
Day Job:
Tribalogic Limited http://www.tribalogic.net/
Open Source:
Mageia Contributor http://www.mageia.org/
PulseAudio Hacker http://www.pulseaudio.org/
Trac Hacker http://trac.edgewall.org/
