2011/8/10 xing wang <[email protected]>: > 2011/8/10 Maarten Bosmans <[email protected]> >> >> Looks good. >> Minor comments below. >> > Thanks Marrten!. > >> >> Maarten >> >> 2011/8/10 <[email protected]>: >> > From: xingchao <[email protected]> >> > >> > 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 <[email protected]> >> > --- >> > 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 _______________________________________________ pulseaudio-discuss mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
