[pulseaudio-discuss] [PATCH] pactl: Fix crash with older servers
Peter Meerwald
pmeerw at pmeerw.net
Tue Mar 18 06:24:46 PDT 2014
Hello Tanu,
> Servers older than 0.9.15 don't know anything about cards, and card
> operations will return a NULL pa_operation object when connected to
> that old server. We must check the pa_operation pointer before passing
> it to pa_operation_unref(), otherwise a NULL operation will result in
> a crash.
I found the purpose of the actions variable quite hard to understand, a
comment would be good explaining the logic
p.
> ---
> src/utils/pactl.c | 201 +++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 132 insertions(+), 69 deletions(-)
>
> diff --git a/src/utils/pactl.c b/src/utils/pactl.c
> index 40e6689..b15b52b 100644
> --- a/src/utils/pactl.c
> +++ b/src/utils/pactl.c
> @@ -93,7 +93,7 @@ static pa_stream *sample_stream = NULL;
> static pa_sample_spec sample_spec;
> static pa_channel_map channel_map;
> static size_t sample_length = 0;
> -static int actions = 1;
> +static int actions = 0;
>
> static bool nl = false;
>
> @@ -1015,6 +1015,7 @@ static void set_sink_formats(pa_context *c, uint32_t sink, const char *str) {
> char *format = NULL;
> const char *state = NULL;
> int i = 0;
> + pa_operation *o = NULL;
>
> while ((format = pa_split(str, ";", &state))) {
> pa_format_info *f = pa_format_info_from_string(pa_strip(format));
> @@ -1028,7 +1029,11 @@ static void set_sink_formats(pa_context *c, uint32_t sink, const char *str) {
> pa_xfree(format);
> }
>
> - pa_operation_unref(pa_ext_device_restore_save_formats(c, PA_DEVICE_TYPE_SINK, sink, i, f_arr, simple_callback, NULL));
> + o = pa_ext_device_restore_save_formats(c, PA_DEVICE_TYPE_SINK, sink, i, f_arr, simple_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
>
> done:
> if (format)
> @@ -1154,7 +1159,10 @@ static void context_subscribe_callback(pa_context *c, pa_subscription_event_type
> }
>
> static void context_state_callback(pa_context *c, void *userdata) {
> + pa_operation *o = NULL;
> +
> pa_assert(c);
> +
> switch (pa_context_get_state(c)) {
> case PA_CONTEXT_CONNECTING:
> case PA_CONTEXT_AUTHORIZING:
> @@ -1164,21 +1172,26 @@ static void context_state_callback(pa_context *c, void *userdata) {
> case PA_CONTEXT_READY:
> switch (action) {
> case STAT:
> - pa_operation_unref(pa_context_stat(c, stat_callback, NULL));
> + o = pa_context_stat(c, stat_callback, NULL);
> if (short_list_format)
> break;
> - actions++;
> +
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> + /* Fall through */
>
> case INFO:
> - pa_operation_unref(pa_context_get_server_info(c, get_server_info_callback, NULL));
> + o = pa_context_get_server_info(c, get_server_info_callback, NULL);
> break;
>
> case PLAY_SAMPLE:
> - pa_operation_unref(pa_context_play_sample(c, sample_name, sink_name, PA_VOLUME_NORM, simple_callback, NULL));
> + o = pa_context_play_sample(c, sample_name, sink_name, PA_VOLUME_NORM, simple_callback, NULL);
> break;
>
> case REMOVE_SAMPLE:
> - pa_operation_unref(pa_context_remove_sample(c, sample_name, simple_callback, NULL));
> + o = pa_context_remove_sample(c, sample_name, simple_callback, NULL);
> break;
>
> case UPLOAD_SAMPLE:
> @@ -1188,163 +1201,203 @@ static void context_state_callback(pa_context *c, void *userdata) {
> pa_stream_set_state_callback(sample_stream, stream_state_callback, NULL);
> pa_stream_set_write_callback(sample_stream, stream_write_callback, NULL);
> pa_stream_connect_upload(sample_stream, sample_length);
> + actions++;
> break;
>
> case EXIT:
> - pa_operation_unref(pa_context_exit_daemon(c, simple_callback, NULL));
> + o = pa_context_exit_daemon(c, simple_callback, NULL);
> break;
>
> case LIST:
> if (list_type) {
> if (pa_streq(list_type, "modules"))
> - pa_operation_unref(pa_context_get_module_info_list(c, get_module_info_callback, NULL));
> + o = pa_context_get_module_info_list(c, get_module_info_callback, NULL);
> else if (pa_streq(list_type, "sinks"))
> - pa_operation_unref(pa_context_get_sink_info_list(c, get_sink_info_callback, NULL));
> + o = pa_context_get_sink_info_list(c, get_sink_info_callback, NULL);
> else if (pa_streq(list_type, "sources"))
> - pa_operation_unref(pa_context_get_source_info_list(c, get_source_info_callback, NULL));
> + o = pa_context_get_source_info_list(c, get_source_info_callback, NULL);
> else if (pa_streq(list_type, "sink-inputs"))
> - pa_operation_unref(pa_context_get_sink_input_info_list(c, get_sink_input_info_callback, NULL));
> + o = pa_context_get_sink_input_info_list(c, get_sink_input_info_callback, NULL);
> else if (pa_streq(list_type, "source-outputs"))
> - pa_operation_unref(pa_context_get_source_output_info_list(c, get_source_output_info_callback, NULL));
> + o = pa_context_get_source_output_info_list(c, get_source_output_info_callback, NULL);
> else if (pa_streq(list_type, "clients"))
> - pa_operation_unref(pa_context_get_client_info_list(c, get_client_info_callback, NULL));
> + o = pa_context_get_client_info_list(c, get_client_info_callback, NULL);
> else if (pa_streq(list_type, "samples"))
> - pa_operation_unref(pa_context_get_sample_info_list(c, get_sample_info_callback, NULL));
> + o = pa_context_get_sample_info_list(c, get_sample_info_callback, NULL);
> else if (pa_streq(list_type, "cards"))
> - pa_operation_unref(pa_context_get_card_info_list(c, get_card_info_callback, NULL));
> + o = pa_context_get_card_info_list(c, get_card_info_callback, NULL);
> else
> pa_assert_not_reached();
> } else {
> - actions = 8;
> - pa_operation_unref(pa_context_get_module_info_list(c, get_module_info_callback, NULL));
> - pa_operation_unref(pa_context_get_sink_info_list(c, get_sink_info_callback, NULL));
> - pa_operation_unref(pa_context_get_source_info_list(c, get_source_info_callback, NULL));
> - pa_operation_unref(pa_context_get_sink_input_info_list(c, get_sink_input_info_callback, NULL));
> - pa_operation_unref(pa_context_get_source_output_info_list(c, get_source_output_info_callback, NULL));
> - pa_operation_unref(pa_context_get_client_info_list(c, get_client_info_callback, NULL));
> - pa_operation_unref(pa_context_get_sample_info_list(c, get_sample_info_callback, NULL));
> - pa_operation_unref(pa_context_get_card_info_list(c, get_card_info_callback, NULL));
> + o = pa_context_get_module_info_list(c, get_module_info_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> +
> + o = pa_context_get_sink_info_list(c, get_sink_info_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> +
> + o = pa_context_get_source_info_list(c, get_source_info_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> + o = pa_context_get_sink_input_info_list(c, get_sink_input_info_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> +
> + o = pa_context_get_source_output_info_list(c, get_source_output_info_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> +
> + o = pa_context_get_client_info_list(c, get_client_info_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> +
> + o = pa_context_get_sample_info_list(c, get_sample_info_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> +
> + o = pa_context_get_card_info_list(c, get_card_info_callback, NULL);
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> +
> + o = NULL;
> }
> break;
>
> case MOVE_SINK_INPUT:
> - pa_operation_unref(pa_context_move_sink_input_by_name(c, sink_input_idx, sink_name, simple_callback, NULL));
> + o = pa_context_move_sink_input_by_name(c, sink_input_idx, sink_name, simple_callback, NULL);
> break;
>
> case MOVE_SOURCE_OUTPUT:
> - pa_operation_unref(pa_context_move_source_output_by_name(c, source_output_idx, source_name, simple_callback, NULL));
> + o = pa_context_move_source_output_by_name(c, source_output_idx, source_name, simple_callback, NULL);
> break;
>
> case LOAD_MODULE:
> - pa_operation_unref(pa_context_load_module(c, module_name, module_args, index_callback, NULL));
> + o = pa_context_load_module(c, module_name, module_args, index_callback, NULL);
> break;
>
> case UNLOAD_MODULE:
> if (module_name)
> - pa_operation_unref(pa_context_get_module_info_list(c, unload_module_by_name_callback, NULL));
> + o = pa_context_get_module_info_list(c, unload_module_by_name_callback, NULL);
> else
> - pa_operation_unref(pa_context_unload_module(c, module_index, simple_callback, NULL));
> + o = pa_context_unload_module(c, module_index, simple_callback, NULL);
> break;
>
> case SUSPEND_SINK:
> if (sink_name)
> - pa_operation_unref(pa_context_suspend_sink_by_name(c, sink_name, suspend, simple_callback, NULL));
> + o = pa_context_suspend_sink_by_name(c, sink_name, suspend, simple_callback, NULL);
> else
> - pa_operation_unref(pa_context_suspend_sink_by_index(c, PA_INVALID_INDEX, suspend, simple_callback, NULL));
> + o = pa_context_suspend_sink_by_index(c, PA_INVALID_INDEX, suspend, simple_callback, NULL);
> break;
>
> case SUSPEND_SOURCE:
> if (source_name)
> - pa_operation_unref(pa_context_suspend_source_by_name(c, source_name, suspend, simple_callback, NULL));
> + o = pa_context_suspend_source_by_name(c, source_name, suspend, simple_callback, NULL);
> else
> - pa_operation_unref(pa_context_suspend_source_by_index(c, PA_INVALID_INDEX, suspend, simple_callback, NULL));
> + o = pa_context_suspend_source_by_index(c, PA_INVALID_INDEX, suspend, simple_callback, NULL);
> break;
>
> case SET_CARD_PROFILE:
> - pa_operation_unref(pa_context_set_card_profile_by_name(c, card_name, profile_name, simple_callback, NULL));
> + o = pa_context_set_card_profile_by_name(c, card_name, profile_name, simple_callback, NULL);
> break;
>
> case SET_SINK_PORT:
> - pa_operation_unref(pa_context_set_sink_port_by_name(c, sink_name, port_name, simple_callback, NULL));
> + o = pa_context_set_sink_port_by_name(c, sink_name, port_name, simple_callback, NULL);
> break;
>
> case SET_DEFAULT_SINK:
> - pa_operation_unref(pa_context_set_default_sink(c, sink_name, simple_callback, NULL));
> + o = pa_context_set_default_sink(c, sink_name, simple_callback, NULL);
> break;
>
> case SET_SOURCE_PORT:
> - pa_operation_unref(pa_context_set_source_port_by_name(c, source_name, port_name, simple_callback, NULL));
> + o = pa_context_set_source_port_by_name(c, source_name, port_name, simple_callback, NULL);
> break;
>
> case SET_DEFAULT_SOURCE:
> - pa_operation_unref(pa_context_set_default_source(c, source_name, simple_callback, NULL));
> + o = pa_context_set_default_source(c, source_name, simple_callback, NULL);
> break;
>
> case SET_SINK_MUTE:
> if (mute == TOGGLE_MUTE)
> - pa_operation_unref(pa_context_get_sink_info_by_name(c, sink_name, sink_toggle_mute_callback, NULL));
> + o = pa_context_get_sink_info_by_name(c, sink_name, sink_toggle_mute_callback, NULL);
> else
> - pa_operation_unref(pa_context_set_sink_mute_by_name(c, sink_name, mute, simple_callback, NULL));
> + o = pa_context_set_sink_mute_by_name(c, sink_name, mute, simple_callback, NULL);
> break;
>
> case SET_SOURCE_MUTE:
> if (mute == TOGGLE_MUTE)
> - pa_operation_unref(pa_context_get_source_info_by_name(c, source_name, source_toggle_mute_callback, NULL));
> + o = pa_context_get_source_info_by_name(c, source_name, source_toggle_mute_callback, NULL);
> else
> - pa_operation_unref(pa_context_set_source_mute_by_name(c, source_name, mute, simple_callback, NULL));
> + o = pa_context_set_source_mute_by_name(c, source_name, mute, simple_callback, NULL);
> break;
>
> case SET_SINK_INPUT_MUTE:
> if (mute == TOGGLE_MUTE)
> - pa_operation_unref(pa_context_get_sink_input_info(c, sink_input_idx, sink_input_toggle_mute_callback, NULL));
> + o = pa_context_get_sink_input_info(c, sink_input_idx, sink_input_toggle_mute_callback, NULL);
> else
> - pa_operation_unref(pa_context_set_sink_input_mute(c, sink_input_idx, mute, simple_callback, NULL));
> + o = pa_context_set_sink_input_mute(c, sink_input_idx, mute, simple_callback, NULL);
> break;
>
> case SET_SOURCE_OUTPUT_MUTE:
> if (mute == TOGGLE_MUTE)
> - pa_operation_unref(pa_context_get_source_output_info(c, source_output_idx, source_output_toggle_mute_callback, NULL));
> + o = pa_context_get_source_output_info(c, source_output_idx, source_output_toggle_mute_callback, NULL);
> else
> - pa_operation_unref(pa_context_set_source_output_mute(c, source_output_idx, mute, simple_callback, NULL));
> + o = pa_context_set_source_output_mute(c, source_output_idx, mute, simple_callback, NULL);
> break;
>
> case SET_SINK_VOLUME:
> if ((volume_flags & VOL_RELATIVE) == VOL_RELATIVE) {
> - pa_operation_unref(pa_context_get_sink_info_by_name(c, sink_name, get_sink_volume_callback, NULL));
> + o = pa_context_get_sink_info_by_name(c, sink_name, get_sink_volume_callback, NULL);
> } else {
> pa_cvolume v;
> pa_cvolume_set(&v, 1, volume);
> - pa_operation_unref(pa_context_set_sink_volume_by_name(c, sink_name, &v, simple_callback, NULL));
> + o = pa_context_set_sink_volume_by_name(c, sink_name, &v, simple_callback, NULL);
> }
> break;
>
> case SET_SOURCE_VOLUME:
> if ((volume_flags & VOL_RELATIVE) == VOL_RELATIVE) {
> - pa_operation_unref(pa_context_get_source_info_by_name(c, source_name, get_source_volume_callback, NULL));
> + o = pa_context_get_source_info_by_name(c, source_name, get_source_volume_callback, NULL);
> } else {
> pa_cvolume v;
> pa_cvolume_set(&v, 1, volume);
> - pa_operation_unref(pa_context_set_source_volume_by_name(c, source_name, &v, simple_callback, NULL));
> + o = pa_context_set_source_volume_by_name(c, source_name, &v, simple_callback, NULL);
> }
> break;
>
> case SET_SINK_INPUT_VOLUME:
> if ((volume_flags & VOL_RELATIVE) == VOL_RELATIVE) {
> - pa_operation_unref(pa_context_get_sink_input_info(c, sink_input_idx, get_sink_input_volume_callback, NULL));
> + o = pa_context_get_sink_input_info(c, sink_input_idx, get_sink_input_volume_callback, NULL);
> } else {
> pa_cvolume v;
> pa_cvolume_set(&v, 1, volume);
> - pa_operation_unref(pa_context_set_sink_input_volume(c, sink_input_idx, &v, simple_callback, NULL));
> + o = pa_context_set_sink_input_volume(c, sink_input_idx, &v, simple_callback, NULL);
> }
> break;
>
> case SET_SOURCE_OUTPUT_VOLUME:
> if ((volume_flags & VOL_RELATIVE) == VOL_RELATIVE) {
> - pa_operation_unref(pa_context_get_source_output_info(c, source_output_idx, get_source_output_volume_callback, NULL));
> + o = pa_context_get_source_output_info(c, source_output_idx, get_source_output_volume_callback, NULL);
> } else {
> pa_cvolume v;
> pa_cvolume_set(&v, 1, volume);
> - pa_operation_unref(pa_context_set_source_output_volume(c, source_output_idx, &v, simple_callback, NULL));
> + o = pa_context_set_source_output_volume(c, source_output_idx, &v, simple_callback, NULL);
> }
> break;
>
> @@ -1353,30 +1406,40 @@ static void context_state_callback(pa_context *c, void *userdata) {
> break;
>
> case SET_PORT_LATENCY_OFFSET:
> - pa_operation_unref(pa_context_set_port_latency_offset(c, card_name, port_name, latency_offset, simple_callback, NULL));
> + o = pa_context_set_port_latency_offset(c, card_name, port_name, latency_offset, simple_callback, NULL);
> break;
>
> case SUBSCRIBE:
> pa_context_set_subscribe_callback(c, context_subscribe_callback, NULL);
>
> - pa_operation_unref(pa_context_subscribe(
> - c,
> - PA_SUBSCRIPTION_MASK_SINK|
> - PA_SUBSCRIPTION_MASK_SOURCE|
> - PA_SUBSCRIPTION_MASK_SINK_INPUT|
> - PA_SUBSCRIPTION_MASK_SOURCE_OUTPUT|
> - PA_SUBSCRIPTION_MASK_MODULE|
> - PA_SUBSCRIPTION_MASK_CLIENT|
> - PA_SUBSCRIPTION_MASK_SAMPLE_CACHE|
> - PA_SUBSCRIPTION_MASK_SERVER|
> - PA_SUBSCRIPTION_MASK_CARD,
> - NULL,
> - NULL));
> + o = pa_context_subscribe(c,
> + PA_SUBSCRIPTION_MASK_SINK|
> + PA_SUBSCRIPTION_MASK_SOURCE|
> + PA_SUBSCRIPTION_MASK_SINK_INPUT|
> + PA_SUBSCRIPTION_MASK_SOURCE_OUTPUT|
> + PA_SUBSCRIPTION_MASK_MODULE|
> + PA_SUBSCRIPTION_MASK_CLIENT|
> + PA_SUBSCRIPTION_MASK_SAMPLE_CACHE|
> + PA_SUBSCRIPTION_MASK_SERVER|
> + PA_SUBSCRIPTION_MASK_CARD,
> + NULL,
> + NULL);
> break;
>
> default:
> pa_assert_not_reached();
> }
> +
> + if (o) {
> + pa_operation_unref(o);
> + actions++;
> + }
> +
> + if (actions == 0) {
> + pa_log("Operation failed: %s", pa_strerror(pa_context_errno(c)));
> + quit(1);
> + }
> +
> break;
>
> case PA_CONTEXT_TERMINATED:
>
--
Peter Meerwald
+43-664-2444418 (mobile)
More information about the pulseaudio-discuss
mailing list