[pulseaudio-discuss] [pulseaudio-commits] src/modules

[David Henningsson]

On 06/28/2013 02:34 PM, Thomas Martitz wrote:
> Am 28.06.2013 10:19, schrieb Tanu Kaskinen:
>> On Fri, 2013-06-28 at 09:47 +0200, David Henningsson wrote:
>>> I'm having *a lot* of different assertion failures [1] in Ubuntu, more
>>> than I have time to fix, and it might not even be the best use of my
>>> time to fix them. And I'm not saying all of these assertions should be
>>> just removed, but certainly many of them would benefit from someone
>>> thinking them through and replacing them with proper error handling
>>> code. Which often are as simple as "if (a == NULL) return;"
>> Of the assertions failures that you have had time to fix, how many have
>> been cases where the fix has been to replace the assertion with proper
>> error handling? I can remember one: the UCM crash when the configuration
>> didn't have channel count set. That was incorrect assertion use, because
>> the assertion trapped an error in configuration, not code.
>>
>
> I agree, assertions and error handling separate beasts. Assertions
> detect programming errors and make them easy to find (and fix).

See my later emails to Tanu about additional practical challenges for 
making programming errors easy to find: the user must report it and 
reach us.

> Crashing
> with backtrace helps that. Error handling should handle user input (or
> other runtime environment scenarios) gracefully so it doesn't crash.
> Crashing with backtrace doesn't help here because the error is not a bug
> and cannot be easily fixed.
>
> So the question is if it's considered a programming error if
> pa_alsa_path_set_add_ports() is called with a NULL parameter, i.e. if
> all call sites are to be expected to not pass NULL.

Okay, so I wrote that code, so then I decided that calling it with a 
NULL parameter was okay and should be handled properly.

> If yes, then the assertion is right even if unconvinient for the user.
> Hiding programming errors in order to please the user is a recipe for
> failure.

For me, software's most important job is to please the user. Software 
without anyone using it is worthless. Hence, that sentence is directly 
contradictory.

Pleasing the user is a recipe for success. Everything else, like e g 
crashing on assertions, is a recipe for failure.

> If no, the change should be reverted.

Ok, I have now reverted the change.

> But you have to make sure
> the call sites can handle the case where this function returns early and
> does nothing.

The callers must already handle that case: the same thing happens if 
ps->paths is empty.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic