| Message ID | 9c014a2f0264db4085efccf537f76e8f1993c27b.1463282640.git.baruch@tkos.co.il |
|---|---|
| State | Rejected |
| Headers | show |
Hi Buildroot list,
On Sun, May 15, 2016 at 06:24:00AM +0300, Baruch Siach wrote:
> static_assert() in from C++. Don't use it in C code.
This is wrong. As upstream developer Jonas Ådahl pointed out, C11 also defines
static_assert as a convenience macro for _Static_assert. uClibc{,-ng} however
that does not provide this macro. Would it make sense to disable libinput for
uClibc{,-ng}?
baruch
Hello Baruch, On Sun, 15 May 2016 08:34:04 +0300, Baruch Siach <baruch@tkos.co.il> wrote: > Hi Buildroot list, > > On Sun, May 15, 2016 at 06:24:00AM +0300, Baruch Siach wrote: > > static_assert() in from C++. Don't use it in C code. > > This is wrong. As upstream developer Jonas Ådahl pointed out, C11 also defines > static_assert as a convenience macro for _Static_assert. uClibc{,-ng} however > that does not provide this macro. Would it make sense to disable libinput for > uClibc{,-ng}? Or honor the second part of Jonas answer [1]: '... or define a no-op fallback when it is not defined.' Seems a less radical solution than disable complete package (plus dependencies) for uClibc because of a minor issue? Regards, Peter [1] https://lists.freedesktop.org/archives/wayland-devel/2016-May/028877.html > > baruch >
Hello Baruch, On Sun, 15 May 2016 09:49:41 +0200, Peter Seiderer <ps.report@gmx.net> wrote: > Hello Baruch, > > On Sun, 15 May 2016 08:34:04 +0300, Baruch Siach <baruch@tkos.co.il> wrote: > > > Hi Buildroot list, > > > > On Sun, May 15, 2016 at 06:24:00AM +0300, Baruch Siach wrote: > > > static_assert() in from C++. Don't use it in C code. > > > > This is wrong. As upstream developer Jonas Ådahl pointed out, C11 also defines > > static_assert as a convenience macro for _Static_assert. uClibc{,-ng} however > > that does not provide this macro. Would it make sense to disable libinput for > > uClibc{,-ng}? > > Or honor the second part of Jonas answer [1]: '... or define a no-op fallback > when it is not defined.' > > Seems a less radical solution than disable complete package (plus dependencies) > for uClibc because of a minor issue? > Thinking more about it, I think your suggested patch is a good enough solution for buildroot purpose, so you can add my Reviewed-by: Peter Seiderer <ps.report@gmx.net> in case you change the patch descriptions to mention it is not a C++ thing but static_assert is from C11 standard [2] (and besides your patch changes the logic from compile time error to possible run time error (dependent on NDEBUG flag)... Regards, Peter [2] http://en.cppreference.com/w/c/error/static_assert > Regards, > Peter > > > [1] https://lists.freedesktop.org/archives/wayland-devel/2016-May/028877.html > > > > > baruch > > > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hi Peter, On Sun, May 15, 2016 at 01:55:05PM +0200, Peter Seiderer wrote: > On Sun, 15 May 2016 09:49:41 +0200, Peter Seiderer <ps.report@gmx.net> > wrote: > > On Sun, 15 May 2016 08:34:04 +0300, Baruch Siach <baruch@tkos.co.il> > > wrote: > > > On Sun, May 15, 2016 at 06:24:00AM +0300, Baruch Siach wrote: > > > > static_assert() in from C++. Don't use it in C code. > > > > > > This is wrong. As upstream developer Jonas Ådahl pointed out, C11 also defines > > > static_assert as a convenience macro for _Static_assert. uClibc{,-ng} however > > > that does not provide this macro. Would it make sense to disable libinput for > > > uClibc{,-ng}? > > > > Or honor the second part of Jonas answer [1]: '... or define a no-op fallback > > when it is not defined.' > > > > Seems a less radical solution than disable complete package (plus dependencies) > > for uClibc because of a minor issue? > > Thinking more about it, I think your suggested patch is a good enough solution > for buildroot purpose, so you can add my > > Reviewed-by: Peter Seiderer <ps.report@gmx.net> > > in case you change the patch descriptions to mention it is not a C++ thing but > static_assert is from C11 standard [2] (and besides your patch changes the logic > from compile time error to possible run time error (dependent on NDEBUG flag)... That alone is a good enough reason to not apply this patch IMO. Upstream has a better suggestion along the line of no-op fallback. I plan to respin with that fix instead Thanks for reviewing, baruch > [2] http://en.cppreference.com/w/c/error/static_assert > > [1] > > https://lists.freedesktop.org/archives/wayland-devel/2016-May/028877.html
diff --git a/package/libinput/0002-tablet-remove-C-static_assert.patch b/package/libinput/0002-tablet-remove-C-static_assert.patch new file mode 100644 index 000000000000..6998c13437a5 --- /dev/null +++ b/package/libinput/0002-tablet-remove-C-static_assert.patch @@ -0,0 +1,39 @@ +From f0a145dea72d1ff2ef6c110c7eaa505bc5ccec02 Mon Sep 17 00:00:00 2001 +From: Baruch Siach <baruch@tkos.co.il> +Date: Sat, 14 May 2016 22:50:38 +0300 +Subject: [PATCH] tablet: remove C++ static_assert + +static_assert() is C++ only. Build may fail with: + + CCLD event-debug +../src/.libs/libinput.so: undefined reference to `static_assert' +collect2: error: ld returned 1 exit status + +Use just assert() like the rest of the code. + +Signed-off-by: Baruch Siach <baruch@tkos.co.il> +--- +Patch status: reported upstream +(https://lists.freedesktop.org/archives/wayland-devel/2016-May/028876.html) + + src/evdev-tablet.c | 4 +--- + 1 file changed, 1 insertion(+), 3 deletions(-) + +diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c +index be828d965552..9c81be434d2f 100644 +--- a/src/evdev-tablet.c ++++ b/src/evdev-tablet.c +@@ -1178,9 +1178,7 @@ static void + tablet_mark_all_axes_changed(struct tablet_dispatch *tablet, + struct libinput_tablet_tool *tool) + { +- static_assert(sizeof(tablet->changed_axes) == +- sizeof(tool->axis_caps), +- "Mismatching array sizes"); ++ assert(sizeof(tablet->changed_axes) == sizeof(tool->axis_caps)); + + memcpy(tablet->changed_axes, + tool->axis_caps, +-- +2.8.1 +
static_assert() in from C++. Don't use it in C code. Fixes: http://autobuild.buildroot.net/results/3eb/3eb32c19f90a5fd8d45a0c36676e015e8278a469/ http://autobuild.buildroot.net/results/184/1844890c65615f1676a85c6fac78937249eee9f1/ http://autobuild.buildroot.net/results/3a3/3a3f8c5624e8019a6eababbf6e7440fdd668f85f/ Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- .../0002-tablet-remove-C-static_assert.patch | 39 ++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 package/libinput/0002-tablet-remove-C-static_assert.patch