[pulseaudio-discuss] [PATCH] alsa-mixer: Improve workaround for Valgrind's false warnings

[David Henningsson]

Also, this patch supersedes Tanuk's patch "alsa: Silence some Valgrind 
warnings" from Jan 2014.

On 2014-09-02 16:11, David Henningsson wrote:
> Valgrind is not correctly handling ALSA TLV syscalls, which leads
> to false warnings, looking like this:
>   "Conditional jump or move depends on uninitialised value(s)"
>
> Unfortunately, alsa-lib itself also uses these values which valgrind
> falsely believe are uninitialized, so not all warnings are removed,
> but this is what we can do from PA until the valgrind bug is fixed.
>
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>   src/modules/alsa/alsa-mixer.c | 61 ++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 52 insertions(+), 9 deletions(-)
>
> diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
> index ead601a..201f835 100644
> --- a/src/modules/alsa/alsa-mixer.c
> +++ b/src/modules/alsa/alsa-mixer.c
> @@ -50,6 +50,58 @@
>   #include "alsa-mixer.h"
>   #include "alsa-util.h"
>
> +#ifdef HAVE_VALGRIND_MEMCHECK_H
> +/* These macros are workarounds for a bug in valgrind, which is not handling the
> + * ALSA TLV syscalls correctly. See
> + * http://valgrind.10908.n7.nabble.com/Missing-ioctl-for-SNDRV-CTL-IOCTL-TLV-READ-td42711.html */
> +
> +static inline int vgfix_get_capture_dB(snd_mixer_elem_t *a, snd_mixer_selem_channel_id_t b, long *c) {
> +    int r = snd_mixer_selem_get_capture_dB(a, b, c);
> +    VALGRIND_MAKE_MEM_DEFINED(c, sizeof(*c));
> +    return r;
> +}
> +
> +static inline int vgfix_get_playback_dB(snd_mixer_elem_t *a, snd_mixer_selem_channel_id_t b, long *c) {
> +    int r = snd_mixer_selem_get_playback_dB(a, b, c);
> +    VALGRIND_MAKE_MEM_DEFINED(c, sizeof(*c));
> +    return r;
> +}
> +
> +static inline int vgfix_ask_capture_vol_dB(snd_mixer_elem_t *a, long b, long *c) {
> +    int r = snd_mixer_selem_ask_capture_vol_dB(a, b, c);
> +    VALGRIND_MAKE_MEM_DEFINED(c, sizeof(*c));
> +    return r;
> +}
> +
> +static inline int vgfix_ask_playback_vol_dB(snd_mixer_elem_t *a, long b, long *c) {
> +    int r = snd_mixer_selem_ask_playback_vol_dB(a, b, c);
> +    VALGRIND_MAKE_MEM_DEFINED(c, sizeof(*c));
> +    return r;
> +}
> +
> +static inline int vgfix_get_capture_dB_range(snd_mixer_elem_t *a, long *b, long *c) {
> +    int r = snd_mixer_selem_get_capture_dB_range(a, b, c);
> +    VALGRIND_MAKE_MEM_DEFINED(b, sizeof(*b));
> +    VALGRIND_MAKE_MEM_DEFINED(c, sizeof(*c));
> +    return r;
> +}
> +
> +static inline int vgfix_get_playback_dB_range(snd_mixer_elem_t *a, long *b, long *c) {
> +    int r = snd_mixer_selem_get_playback_dB_range(a, b, c);
> +    VALGRIND_MAKE_MEM_DEFINED(b, sizeof(*b));
> +    VALGRIND_MAKE_MEM_DEFINED(c, sizeof(*c));
> +    return r;
> +}
> +
> +#define snd_mixer_selem_get_capture_dB(a, b, c) vgfix_get_capture_dB(a, b, c)
> +#define snd_mixer_selem_get_playback_dB(a, b, c) vgfix_get_playback_dB(a, b, c)
> +#define snd_mixer_selem_ask_capture_vol_dB(a, b, c) vgfix_ask_capture_vol_dB(a, b, c)
> +#define snd_mixer_selem_ask_playback_vol_dB(a, b, c) vgfix_ask_playback_vol_dB(a, b, c)
> +#define snd_mixer_selem_get_capture_dB_range(a, b, c) vgfix_get_capture_dB_range(a, b, c)
> +#define snd_mixer_selem_get_playback_dB_range(a, b, c) vgfix_get_playback_dB_range(a, b, c)
> +
> +#endif
> +
>   static int setting_select(pa_alsa_setting *s, snd_mixer_t *m);
>
>   struct description_map {
> @@ -1001,10 +1053,6 @@ static int element_set_volume(pa_alsa_element *e, snd_mixer_t *m, const pa_chann
>               if (r < 0)
>                   continue;
>
> -#ifdef HAVE_VALGRIND_MEMCHECK_H
> -            VALGRIND_MAKE_MEM_DEFINED(&value, sizeof(value));
> -#endif
> -
>               f = from_alsa_dB(value);
>
>           } else {
> @@ -1517,11 +1565,6 @@ static int element_probe(pa_alsa_element *e, snd_mixer_t *m) {
>                   }
>
>                   if (e->has_dB) {
> -#ifdef HAVE_VALGRIND_MEMCHECK_H
> -                    VALGRIND_MAKE_MEM_DEFINED(&min_dB, sizeof(min_dB));
> -                    VALGRIND_MAKE_MEM_DEFINED(&max_dB, sizeof(max_dB));
> -#endif
> -
>                       e->min_dB = ((double) min_dB) / 100.0;
>                       e->max_dB = ((double) max_dB) / 100.0;
>
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic