Looks good.
Minor comments below.
Maarten
2011/8/10 <wangxingchao2011 at gmail.com>:
> From: xingchao <xingchao.wang at intel.com>
>
> some sound app based on pulseaudio get crashed when setting volume, the coredump
> show it's null pointer in pa_operation_ref().
Don't you mean pa_operation_unref()?
> Signed-off-by: xingchao <xingchao.wang at intel.com>
> ---
> ?src/utils/pactl.c | ? 10 ++++++++--
> ?1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/utils/pactl.c b/src/utils/pactl.c
> index 2eb0f25..487cd41 100644
> --- a/src/utils/pactl.c
> +++ b/src/utils/pactl.c
> @@ -1036,6 +1036,7 @@ static void volume_relative_adjust(pa_cvolume *cv) {
>
> ?static void get_sink_volume_callback(pa_context *c, const pa_sink_info *i, int is_last, void *userdata) {
> ? ? pa_cvolume cv;
> + ? ? ? pa_operation *o;
>
> ? ? if (is_last < 0) {
> ? ? ? ? pa_log(_("Failed to get sink information: %s"), pa_strerror(pa_context_errno(c)));
> @@ -1050,7 +1051,9 @@ static void get_sink_volume_callback(pa_context *c, const pa_sink_info *i, int i
>
> ? ? cv = i->volume;
> ? ? volume_relative_adjust(&cv);
> - ? ?pa_operation_unref(pa_context_set_sink_volume_by_name(c, sink_name, &cv, simple_callback, NULL));
> + ? ? ? o = pa_context_set_sink_volume_by_name(c, sink_name, &cv, simple_callback, NULL);
> + ? ? ? if (o)
> + ? ? ? ? ? pa_operation_unref(o);
> ?}
>
> ?static void get_source_volume_callback(pa_context *c, const pa_source_info *i, int is_last, void *userdata) {
> @@ -1370,7 +1373,10 @@ static void context_state_callback(pa_context *c, void *userdata) {
> ? ? ? ? ? ? ? ? ? ? } 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));
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pa_operation *o = pa_context_set_sink_volume_by_name(c, sink_name, &v, simple_callback, NULL);
You should make this two separate lines to avoid mixing declarations
and code, just like you did in the two hunks above.
> +
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (o)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pa_operation_unref(o);
> ? ? ? ? ? ? ? ? ? ? }
> ? ? ? ? ? ? ? ? ? ? break;
>
> --
> 1.7.1
>
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
>
