Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210912204839.3018089-1-fontaine.fabrice@gmail.com>
In-Reply-To: <20210912204839.3018089-1-fontaine.fabrice@gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Fri, 17 Sep 2021 13:53:33 -0700
Message-ID: <CABBYNZ+Lj1r4NTPuL1RorkYn8ZuTsKAL_oOTKuf40vyOtbW7zQ@mail.gmail.com>
Subject: Re: [PATCH] profiles/audio/media.c: fix build with glibc >= 2.34
To:     Fabrice Fontaine <fontaine.fabrice@gmail.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "Yann E . MORIN" <yann.morin.1998@free.fr>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Fabrice,

On Sun, Sep 12, 2021 at 1:50 PM Fabrice Fontaine
<fontaine.fabrice@gmail.com> wrote:
>
> pause() is defined in glibc since the very early times; it appears in
> upstream commit 28f540f45bba (initial import) in 1995 [0].
>
> Bluez has been defining a function named pause() for ages too, since
> commit caab74c97542 (media: Implement new callbacks for pass-through
> operations) in 2013 [1]
>
> With the recent bump to glibc 2.34.xxx, the build now fails because the
> two pause() clash:
>
> profiles/audio/media.c:1284:13: error: conflicting types for 'pause'
>  1284 | static bool pause(void *user_data)
>       |             ^~~~~
> In file included from /tmp/instance-0/output-1/per-package/bluez5_utils/host/s390x-buildroot-linux-gnu/sysroot/usr/include/bits/sigstksz.h:24,
>                  from /tmp/instance-0/output-1/per-package/bluez5_utils/host/s390x-buildroot-linux-gnu/sysroot/usr/include/signal.h:328,
>                  from /tmp/instance-0/output-1/per-package/bluez5_utils/host/bin/../s390x-buildroot-linux-gnu/sysroot/usr/include/glib-2.0/glib/gbacktrace.h:36,
>                  from /tmp/instance-0/output-1/per-package/bluez5_utils/host/bin/../s390x-buildroot-linux-gnu/sysroot/usr/include/glib-2.0/glib.h:34,
>                  from profiles/audio/media.c:21:
> /tmp/instance-0/output-1/per-package/bluez5_utils/host/s390x-buildroot-linux-gnu/sysroot/usr/include/unistd.h:489:12: note: previous declaration of 'pause' was here
>   489 | extern int pause (void);
>       |            ^~~~~
>
> The culprit is indeed glibc 2.34, as can be seen in this result matrix:
>
>          \   bluez5_utils
>     glibc \  5.60  |  5.61
>     -------\-------+--------
>     2.33   |  OK   |   OK
>     -------+-------+--------
>     2.34   |  KO   |   KO
>
> The underlying reason that pause() is now causing issues with glibc 2.34
> is not obvious: glibc is a big beast, and finding such issues is not
> easy. However, we can see that the pause() provided by NPTL has been
> dropped in favour of the generic one, so maybe this is causing symbol
> visibility or weakness to change or something...
>
> We fix that by renaming the local pause() in bluez5_utils with a
> namespace-prefix, like some other functions there already have.
>
> Fixes:
>   - http://autobuild.buildroot.org/results/c4fbface34be8815838fd7201621d7a8fddd32c5
>   - http://autobuild.buildroot.org/results/62b88740f19fbe4a1ad7959dc141d539eb88c1f8
>
> [0] https://sourceware.org/git/?p=glibc.git;a=commit;h=28f540f45bbacd939bfd07f213bcad2bf730b1bf
> [1] https://github.com/bluez/bluez/commit/caab74c97542a56b591f0b16b44ab6ba4b40f0f5
>
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
>  profiles/audio/media.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index 267722542..f93b74e67 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -1281,7 +1281,7 @@ static bool stop(void *user_data)
>         return media_player_send(mp, "Stop");
>  }
>
> -static bool pause(void *user_data)
> +static bool media_player_pause(void *user_data)
>  {
>         struct media_player *mp = user_data;
>
> @@ -1331,7 +1331,7 @@ static struct avrcp_player_cb player_cb = {
>         .set_volume = set_volume,
>         .play = play,
>         .stop = stop,
> -       .pause = pause,
> +       .pause = media_player_pause,
>         .next = next,
>         .previous = previous,
>  };
> --
> 2.33.0

Applied, thanks.

-- 
Luiz Augusto von Dentz