[pulseaudio-discuss] [RFC][PATCH] Initialise write_volume callback

[Tanu Kaskinen]

On Sat, 2011-08-13 at 16:57 +0200, Maarten Bosmans wrote:
> Hi list,
> 
> I was getting some warnings about possible uninitialised variables in
> pulsecore/{source,sink}.c. At first the solution seemed obvious: just
> set the callback to NULL in reset_callbacks, as it looks like it was
> forgotten to be added there when write_volume was added.

Yes, it was most likely forgotten.

> The attached patch does this and indeed solves the warning about
> reading from uninitialised memory. But it triggers an assert:
> E: sink.c: Assertion 's->write_volume' failed at
> pulsecore/sink.c:3353, function pa_sink_volume_change_apply().
> Aborting.

This happens when calling pa_sink_unlink() but not when calling
pa_sink_new(), right?

> This can be worked around by not adding the line "s->write_volume =
> NULL" to reset_callbacks(), but do after reset_callbacks() is called
> in pa_sink_new() and not adding it at the other place
> reset_callbacks() is called, pa_sink_unlink().
> 
> However adding the initialisatino to reset_callbacks seems better to
> me. Does anyone have a suggestion how to fix this?

So the problem is that after unlinking the sink, there may still be
pending volume changes. The crash could be worked around, in addition to
your solution, by removing the assertion and calling s->write_volume()
only if it's set, or by removing reset_callbacks() from
pa_sink_unlink(), or by adding a check to the beginning of
pa_sink_volume_change_apply() that returns immediately if the sink is
not linked.

What would be the correct solution? I think the sink implementation
(alsa sink in this case, I assume) should tear down the "audio
link" (ie. close the alsa device in this case) already when the sink
state gets changed to unlinked. Currently at least the alsa sink closes
the device only after pa_sink_unlink() has finished. If we can assume
that the device is closed after the sink implementation has had the
chance to do it in the PA_SINK_MESSAGE_SET_STATE handler, the pending
volume changes can't do anything useful anymore, so we can drop all
pending volume changes in the PA_SINK_MESSAGE_SET_STATE handler of
pa_sink_process_msg().

-- 
Tanu