[pulseaudio-discuss] [PATCH] null pointer check when setting volume

[Maarten Bosmans]

2011/8/10 xing wang <wangxingchao2011 at gmail.com>:
> 2011/8/10 Maarten Bosmans <mkbosmans at gmail.com>
>>
>> Looks good.
>> Minor comments below.
>>
> Thanks Marrten!.
>
>>
>> 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()?
>>
>
> Oh, sorry for confuse, you're right, it's 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.
>
> Seems there's some pattern issue when send out the patch with git
> send-email. Please find attached patch, it should be okay.
> --xingchao

No, the patch came through all right. What I meant was that you change

pa_cvolume v;
pa_cvolume_set(&v, 1, volume);
pa_operation *o = pa_context_set_sink_volume_by_name(...);

into

pa_cvolume v;
pa_operation *o;
pa_cvolume_set(&v, 1, volume);
o = pa_context_set_sink_volume_by_name(...);

I find the second approach of separating declaration and code cleaner,
but perhaps others disagree. http://pulseaudio.org/wiki/CodingStyle
does not say anything specific about this issue, so either way would
probably go.

Maarten