Skip to contentSkip to conversation

package/canfestival: fix build with musl

PATCH New Yann E. MORIN 4 participants 7 messages Aug 22, 2015 – Jan 16, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 package/canfestival: fix build with musl

---
 package/canfestival/0003-sigval-musl.patch | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 package/canfestival/0003-sigval-musl.patch
package/canfestival/0003-sigval-musl.patch new file +20 -0
new file mode 100644
index 0000000..7a4dadf
@@ -0,0 +1,20 @@
+timers/unix: fix build with musl
+
+sigval_t is a glibcism, which is not defined in musl.
+
+Use the union instead.
+
+Signed-off-by: "Yann E. MORIN" <[email protected]>
+
+diff -durN a/drivers/timers_unix/timers_unix.c b/drivers/timers_unix/timers_unix.c
+--- a/drivers/timers_unix/timers_unix.c	2014-06-12 14:07:16.000000000 +0200
++++ b/drivers/timers_unix/timers_unix.c	2015-08-22 23:12:26.921455179 +0200
+@@ -33,7 +33,7 @@
+ 	}
+ }
+ 
+-void timer_notify(sigval_t val)
++void timer_notify(union sigval val)
+ {
+ 	if(gettimeofday(&last_sig,NULL)) {
+ 		perror("gettimeofday()");
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 package/canfestival: fix build with musl

Hello All!

On 2015-08-22 23:35 +0200, Yann E. MORIN spake thusly:

Quoted 1 line

Damn, I forgot the autobuilder refs:

Fixes:

    http://autobuild.buildroot.org/results/8eb/8ebb38f4b1ab4d1eadf7139e3467ed59ccb10646/
    http://autobuild.buildroot.org/results/0da/0dac4c0639d75540e004d0fdd0ac6cd9fb9c7ef0/
    http://autobuild.buildroot.org/results/75a/75aa6e7e4f3dcd74e4b65496060328ec7bd0e747/
    ...

Regards,
Yann E. MORIN.

Quoted 34 lines
---
 package/canfestival/0003-sigval-musl.patch | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 package/canfestival/0003-sigval-musl.patch
package/canfestival/0003-sigval-musl.patch new file +20 -0
new file mode 100644
index 0000000..7a4dadf
@@ -0,0 +1,20 @@
+timers/unix: fix build with musl
+
+sigval_t is a glibcism, which is not defined in musl.
+
+Use the union instead.
+
+Signed-off-by: "Yann E. MORIN" <[email protected]>
+
+diff -durN a/drivers/timers_unix/timers_unix.c b/drivers/timers_unix/timers_unix.c
+--- a/drivers/timers_unix/timers_unix.c	2014-06-12 14:07:16.000000000 +0200
++++ b/drivers/timers_unix/timers_unix.c	2015-08-22 23:12:26.921455179 +0200
+@@ -33,7 +33,7 @@
+ 	}
+ }
+ 
+-void timer_notify(sigval_t val)
++void timer_notify(union sigval val)
+ {
+ 	if(gettimeofday(&last_sig,NULL)) {
+ 		perror("gettimeofday()");
Signature · 1 line
1.9.1
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#2 lore raw

PATCH 1/1 package/canfestival: fix build with musl

Yann,

I submitted a patch similar to this about a week ago. Not sure if you spotted it or not.

When signal() is being used (as it will be when __UCLIBC__ is defined) the handler should take an int as a parameter, not a sigval_t.

I was going to take this up with the developers as I can't see a good reason why the behaviour is different for uclibc and non-uclibc environments, and in any case CLOCK_REALTIME is almost certainly not what they want. But I haven't had a spare moment to write a test case to prove that the timer code used in the non-uclibc version would work just as well under uclibc.

regards

Brendan

Quoted 62 lines · Yann E. MORIN

Hello All!

On 2015-08-22 23:35 +0200, Yann E. MORIN spake thusly:

Quoted 1 line

Damn, I forgot the autobuilder refs:

Fixes:

    http://autobuild.buildroot.org/results/8eb/8ebb38f4b1ab4d1eadf7139e3467ed59ccb10646/
    http://autobuild.buildroot.org/results/0da/0dac4c0639d75540e004d0fdd0ac6cd9fb9c7ef0/
    http://autobuild.buildroot.org/results/75a/75aa6e7e4f3dcd74e4b65496060328ec7bd0e747/
    ...

Regards,
Yann E. MORIN.

Quoted 32 lines
---
 package/canfestival/0003-sigval-musl.patch | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 package/canfestival/0003-sigval-musl.patch
package/canfestival/0003-sigval-musl.patch new file +20 -1
new file mode 100644
index 0000000..7a4dadf
@@ -0,0 +1,20 @@
+timers/unix: fix build with musl
+
+sigval_t is a glibcism, which is not defined in musl.
+
+Use the union instead.
+
+Signed-off-by: "Yann E. MORIN" <[email protected]>
+
+diff -durN a/drivers/timers_unix/timers_unix.c b/drivers/timers_unix/timers_unix.c
+--- a/drivers/timers_unix/timers_unix.c      2014-06-12 14:07:16.000000000 +0200
++++ b/drivers/timers_unix/timers_unix.c      2015-08-22 23:12:26.921455179 +0200
+@@ -33,7 +33,7 @@
+     }
+ }
+
+-void timer_notify(sigval_t val)
++void timer_notify(union sigval val)
+ {
+     if(gettimeofday(&last_sig,NULL)) {
+             perror("gettimeofday()");
--
Quoted 2 lines

1.9.1

--
.-----------------.--------------------.------------------.--------------------.

|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |

'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

#3 lore raw

PATCH 1/1 package/canfestival: fix build with musl

Brendan, All,

On 2015-08-22 22:45 +0100, Brendan Heading spake thusly:

Quoted 2 lines

I submitted a patch similar to this about a week ago. Not sure if you spotted it or not.

Ah, no. I looked at the patchwork, but there was no canfestival-related patch. But now, I see that it was marked rejected, hence the reason I did not see it.

Quoted 9 lines

When signal() is being used (as it will be when __UCLIBC__ is defined) the handler should take an int as a parameter, not a sigval_t.

I was going to take this up with the developers as I can't see a good reason why the behaviour is different for uclibc and non-uclibc environments, and in any case CLOCK_REALTIME is almost certainly not what they want. But I haven't had a spare moment to write a test case to prove that the timer code used in the non-uclibc version would work just as well under uclibc.

That sounds really weird, indeed... :-(

OK, I'll try to revisit this later. Thanks for the heads up.

Regards,
Yann E. MORIN.

Quoted 69 lines

regards

Brendan

Quoted 62 lines · Yann E. MORIN

Hello All!

On 2015-08-22 23:35 +0200, Yann E. MORIN spake thusly:

Quoted 1 line

Damn, I forgot the autobuilder refs:

Fixes:

    http://autobuild.buildroot.org/results/8eb/8ebb38f4b1ab4d1eadf7139e3467ed59ccb10646/
    http://autobuild.buildroot.org/results/0da/0dac4c0639d75540e004d0fdd0ac6cd9fb9c7ef0/
    http://autobuild.buildroot.org/results/75a/75aa6e7e4f3dcd74e4b65496060328ec7bd0e747/
    ...

Regards,
Yann E. MORIN.

Quoted 32 lines
---
 package/canfestival/0003-sigval-musl.patch | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 package/canfestival/0003-sigval-musl.patch
package/canfestival/0003-sigval-musl.patch new file +20 -1
new file mode 100644
index 0000000..7a4dadf
@@ -0,0 +1,20 @@
+timers/unix: fix build with musl
+
+sigval_t is a glibcism, which is not defined in musl.
+
+Use the union instead.
+
+Signed-off-by: "Yann E. MORIN" <[email protected]>
+
+diff -durN a/drivers/timers_unix/timers_unix.c b/drivers/timers_unix/timers_unix.c
+--- a/drivers/timers_unix/timers_unix.c      2014-06-12 14:07:16.000000000 +0200
++++ b/drivers/timers_unix/timers_unix.c      2015-08-22 23:12:26.921455179 +0200
+@@ -33,7 +33,7 @@
+     }
+ }
+
+-void timer_notify(sigval_t val)
++void timer_notify(union sigval val)
+ {
+     if(gettimeofday(&last_sig,NULL)) {
+             perror("gettimeofday()");
--
Quoted 2 lines

1.9.1

--
.-----------------.--------------------.------------------.--------------------.

|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |

'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#4 lore raw

PATCH 1/1 package/canfestival: fix build with musl

Quoted 8 lines
Quoted 6 lines

I was going to take this up with the developers as I can't see a good reason why the behaviour is different for uclibc and non-uclibc environments, and in any case CLOCK_REALTIME is almost certainly not what they want. But I haven't had a spare moment to write a test case to prove that the timer code used in the non-uclibc version would work just as well under uclibc.

That sounds really weird, indeed... :-(

It feels like they ran into some kind of issue with uclibc and hacked together a fix.

And IME naive coders often make the mistake of using the wrong CLOCK_* parameters when doing POSIX time related things ..

Quoted 1 line

OK, I'll try to revisit this later. Thanks for the heads up.

I thought about submitting a patch with another #ifdef __UCLIBC__ around the function declaration - not ideal, but it at least matches what the upstream were trying to do. But I wasn't sure if such an approach would pass muster here ?

Brendan

#5 lore raw

PATCH 1/1 package/canfestival: fix build with musl

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Brendan" == Brendan Heading <[email protected]> writes:

Hi,

Quoted 4 lines

I thought about submitting a patch with another #ifdef __UCLIBC__ around the function declaration - not ideal, but it at least matches what the upstream were trying to do. But I wasn't sure if such an approach would pass muster here ?

It would be good to get it acked by upstream / included in the git repo so we can instead just bump the version.

Signature · 1 line
Bye, Peter Korsgaard
#6 lore raw

PATCH 1/1 package/canfestival: fix build with musl

In reply to Brendan Heading

Brendan, Yann,

Quoted 15 lines · Brendan Heading
Quoted 8 lines
Quoted 6 lines

I was going to take this up with the developers as I can't see a good reason why the behaviour is different for uclibc and non-uclibc environments, and in any case CLOCK_REALTIME is almost certainly not what they want. But I haven't had a spare moment to write a test case to prove that the timer code used in the non-uclibc version would work just as well under uclibc.

That sounds really weird, indeed... :-(

It feels like they ran into some kind of issue with uclibc and hacked together a fix.

And IME naive coders often make the mistake of using the wrong CLOCK_* parameters when doing POSIX time related things ..

Quoted 6 lines
Quoted 1 line

OK, I'll try to revisit this later. Thanks for the heads up.

I thought about submitting a patch with another #ifdef __UCLIBC__ around the function declaration - not ideal, but it at least matches what the upstream were trying to do. But I wasn't sure if such an approach would pass muster here ?

Any progress with this? can-festival still fails to build in the autobuilders due to this:

http://autobuild.buildroot.net/results/5c5a48b050223d5f4f66a3a5ddde12cb17e310d7/

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com