linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] profiles/audio/media.c: fix build with glibc >= 2.34
@ 2021-09-12 20:48 Fabrice Fontaine
  2021-09-12 21:31 ` bluez.test.bot
  2021-09-17 20:53 ` [PATCH] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 3+ messages in thread
From: Fabrice Fontaine @ 2021-09-12 20:48 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Fabrice Fontaine, Yann E . MORIN

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: profiles/audio/media.c: fix build with glibc >= 2.34
  2021-09-12 20:48 [PATCH] profiles/audio/media.c: fix build with glibc >= 2.34 Fabrice Fontaine
@ 2021-09-12 21:31 ` bluez.test.bot
  2021-09-17 20:53 ` [PATCH] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2021-09-12 21:31 UTC (permalink / raw)
  To: linux-bluetooth, fontaine.fabrice

[-- Attachment #1: Type: text/plain, Size: 4723 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=545463

---Test result---

Test Summary:
CheckPatch                    FAIL      0.28 seconds
GitLint                       FAIL      0.10 seconds
Prep - Setup ELL              PASS      40.46 seconds
Build - Prep                  PASS      0.10 seconds
Build - Configure             PASS      7.03 seconds
Build - Make                  PASS      174.80 seconds
Make Check                    PASS      8.67 seconds
Make Distcheck                PASS      205.44 seconds
Build w/ext ELL - Configure   PASS      7.10 seconds
Build w/ext ELL - Make        PASS      164.18 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
profiles/audio/media.c: fix build with glibc >= 2.34
WARNING:UNKNOWN_COMMIT_ID: Unknown commit id '28f540f45bba', maybe rebased or not pulled?
#7: 
upstream commit 28f540f45bba (initial import) in 1995 [0].

WARNING:UNKNOWN_COMMIT_ID: Unknown commit id 'caab74c97542', maybe rebased or not pulled?
#10: 
commit caab74c97542 (media: Implement new callbacks for pass-through

WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#19: 
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,

- total: 0 errors, 3 warnings, 16 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] profiles/audio/media.c: fix build with glibc >= 2.34" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint with rule in .gitlint
Output:
profiles/audio/media.c: fix build with glibc >= 2.34
16: B1 Line exceeds max length (142>80): "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,"
17: B1 Line exceeds max length (136>80): "                 from /tmp/instance-0/output-1/per-package/bluez5_utils/host/s390x-buildroot-linux-gnu/sysroot/usr/include/signal.h:328,"
18: B1 Line exceeds max length (160>80): "                 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,"
19: B1 Line exceeds max length (149>80): "                 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,"
21: B1 Line exceeds max length (164>80): "/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"
44: B1 Line exceeds max length (83>80): "  - http://autobuild.buildroot.org/results/c4fbface34be8815838fd7201621d7a8fddd32c5"
45: B1 Line exceeds max length (83>80): "  - http://autobuild.buildroot.org/results/62b88740f19fbe4a1ad7959dc141d539eb88c1f8"
47: B1 Line exceeds max length (95>80): "[0] https://sourceware.org/git/?p=glibc.git;a=commit;h=28f540f45bbacd939bfd07f213bcad2bf730b1bf"
48: B1 Line exceeds max length (82>80): "[1] https://github.com/bluez/bluez/commit/caab74c97542a56b591f0b16b44ab6ba4b40f0f5"


##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] profiles/audio/media.c: fix build with glibc >= 2.34
  2021-09-12 20:48 [PATCH] profiles/audio/media.c: fix build with glibc >= 2.34 Fabrice Fontaine
  2021-09-12 21:31 ` bluez.test.bot
@ 2021-09-17 20:53 ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2021-09-17 20:53 UTC (permalink / raw)
  To: Fabrice Fontaine; +Cc: linux-bluetooth, Yann E . MORIN

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-09-17 20:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-12 20:48 [PATCH] profiles/audio/media.c: fix build with glibc >= 2.34 Fabrice Fontaine
2021-09-12 21:31 ` bluez.test.bot
2021-09-17 20:53 ` [PATCH] " Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).