I was looking at a log that showed that a suspend happened (at
a strange time), but the log didn't tell me why the suspend was done.
This patch tries to make sure that that won't happen again.
In practice there is always at least one profile, and I
don't think there will ever be cards without profiles.
Therefore, I added assertions to pa_card_new() stating that
the card new data must always contain at least one profile.
Now a lot of code can be simplified, because it's guaranteed
that the profiles hashmap and the active_profile field are
always non-NULL.
Coverity thinks that sample can be NULL when it's
dereferenced after this line. Adding an assertion doesn't
hurt here (in my opinion), and that should get rid of the
warning.
If module-dbus-protocol fails to start, pa__done is still called,
which falsified the assumption that u->connections was always set.
BugLink: http://bugs.launchpad.net/bugs/855729
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
The order of freeing the hashmaps is important here, because otherwise a string used as key is freed before the hashmap
is freed.
Valgrind reports this as:
Invalid read of size 1
at 0x4107042: pa_idxset_string_hash_func (idxset.c:67)
by 0x4106026: remove_entry (hashmap.c:93)
by 0x41061BF: pa_hashmap_free (hashmap.c:110)
by 0x71DD143: pa_dbusiface_core_free (iface-core.c:2105)
by 0x71F2169: module_dbus_protocol_LTX_pa__done (module-dbus-protocol.c:595)
by 0x406DC51: pa_module_free (module.c:162)
by 0x406E01D: pa_module_unload_all (module.c:210)
by 0x4068842: core_free (core.c:169)
by 0x406FD5D: pa_object_unref (object.c:64)
by 0x805224D: pa_core_unref (core.h:184)
by 0x805560B: main (main.c:1159)
Address 0x4d099c0 is 0 bytes inside a block of size 100 free'd
at 0x4025BF0: free (vg_replace_malloc.c:366)
by 0x40F128C: pa_xfree (xmalloc.c:131)
by 0x71E4CEB: pa_dbusiface_device_free (iface-device.c:1293)
by 0x71DCD7E: free_device_cb (iface-core.c:2062)
by 0x41061D7: pa_hashmap_free (hashmap.c:113)
by 0x71DD125: pa_dbusiface_core_free (iface-core.c:2104)
by 0x71F2169: module_dbus_protocol_LTX_pa__done (module-dbus-protocol.c:595)
by 0x406DC51: pa_module_free (module.c:162)
by 0x406E01D: pa_module_unload_all (module.c:210)
by 0x4068842: core_free (core.c:169)
by 0x406FD5D: pa_object_unref (object.c:64)
by 0x805224D: pa_core_unref (core.h:184)
Note in protocol-dbus.c specifically, method_signatures needs to be freed
before method_handlers, because otherwise h->method_name is freed while it is
still in use as a key in the method_signatures hashmap.
This piggy backs onto the previous changes for protocol 22 and
thus does not bump the version. This and the previous commits should be
seen as mostly atomic. Apologies for any bisecting issues this causes
(although I would expect these to be minimal)
If u->connections isn't empty when module-dbus-protocol is
unloaded, then connection_free() is called for the
remaining connections when the idxset is freed.
connection_free() tries to remove the connection from the
idxset, but that fails, because the item has already been
removed from the idxset in this scenario.
The problem is solved by not trying to remove the connection
from the idxset in connection_free(). Instead, whoever wants
to delete connections, has to remove the connection from the
idxset in addition to calling connection_free().
This is pretty cosmetic change; there's no actual functionality added.
Previously the volume_writable information was available through the
pa_sink_input_is_volume_writable() function, but I find it cleaner to have a
real variable.
The sink input introspection variable name was also changed from
read_only_volume to volume_writable for consistency.
We should not call pa_core_ref() anywhere in the code. Doing so
will prevent proper daemon shutdown as the only call (in daemon/main.c)
to pa_core_unref() should always call free_core() and perform a normal
shutdown (i.e. unload all modules gracefully).
There are two known cases where read-only or non-existing sink input volume is
relevant: passthrough streams and the planned volume sharing logic.
Passthrough streams don't have volume at all, and the volume sharing logic
requires read-only sink input volume. This commit is primarily working towards
the volume sharing feature, but support for non-existing sink input volume is
also added, because it is so closely related to read-only volume.
Some unrelated refactoring in iface-stream.c creeped into this commit too (new
function: stream_to_string()).
This adds a PA_VOLUME_IS_VALID() macro for checking if a given
pa_volume_t is valid. This makes changes to the volume ranges simpler
(just change PA_VOLUME_MAX, for example, without needing to modify any
other code).
Using the subscription events caused an assertion crash sometimes when a sink
was removed and a new sink was created (i.e. card profile change) and a stream
was moved from the removed sink to the new sink. The stream dbus object's
subscription callback got a change event before the core dbus object's
subscription callback got the sink remove/creation events. The stream's
subscription callback then queried the core for the object path of the new
sink, and since the core was not yet aware of the new sink, an assertion was
hit in pa_dbusiface_device_get_path().
Now that the core uses synchronous hooks to keep the sink and source lists up
to date, this particular problem can't occur anymore.
