[PATCH] pactl: Fix crash with older servers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux