Delete refactor

[derselbst]

refactor all destructor functions to return void if possible and make them safe when called with NULL

[derselbst]

@mawe42 Need a second opinion. This PR ought to refactor (nearly) all destructor functions to return void if possible and make them safe when called with NULL. For consistency I used fluid_return_if_fail which is basically g_return_if_fail to perform the NULL checks. However g_return_... prints an error message when the condition fails (i.e. == NULL). This turns out to be very spammy esp. for delete_fluid_list(). Now should we define fluid_return_if_fail our self that doesnt log msgs or should I just use usual if(obj==NULL)return; statements inside delete_*()?

[mawe42]

Well, it seems like g_return_if_fail is a custom assertion defined by glib. So all code using fluid_return_if_fail can be switched off by setting G_DISABLE_CHECKS. The glib docs say:

If G_DISABLE_CHECKS is defined then the check is not performed. You should therefore not depend on any side effects of expr .
https://developer.gnome.org/glib/stable/glib-Warnings-and-Assertions.html#g-return-if-fail

So if your aim is to make the code safe, then using something that can be disabled by a compile-time switch is not really suitable, IMHO.

[mawe42]

But I think that doesn't quite answer your question. I really like the simple and explicit

if (obj == NULL) 
{
    return NULL;
}

But I don't like that fact that these types of checks take such a large amount of vertical space when using Allman style braces. 5 lines of code for a single check (if you also add an empty line between)... and when you need two or three of these, then a lot of your screen is taken up by that code.

I personally would like either single line checks:

if (obj == NULL) return;

or using a macro, but with an easier to understand logic:

FLUID_RETURN_IF(obj == NULL);

[derselbst]

fluid_return_if_fail is also used throughout the synth API and yes defining G_DISABLE_CHECKS would break quite a lot.

5 lines of code for a single check

I understand your point. On the other hand defining exceptions for this rule might become difficult to enforce. So I would prefer to go with a macro, i.e. by defining fluid_return_if_fail our self rather than introducing yet another fluid_return* macro. A simple

#define fluid_return_if_fail(cond) \
if (cond) \
{} \
else \
return

Should be sufficient. I dont think we need __builtin_expect as glib does it.