| Message ID | 1440279318-19132-1-git-send-email-yann.morin.1998@free.fr |
|---|---|
| State | Rejected |
| Headers | show |
Hello All! On 2015-08-22 23:35 +0200, Yann E. MORIN spake thusly: > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> 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. > --- > package/canfestival/0003-sigval-musl.patch | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > create mode 100644 package/canfestival/0003-sigval-musl.patch > > diff --git a/package/canfestival/0003-sigval-musl.patch b/package/canfestival/0003-sigval-musl.patch > new file mode 100644 > index 0000000..7a4dadf > --- /dev/null > +++ b/package/canfestival/0003-sigval-musl.patch > @@ -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" <yann.morin.1998@free.fr> > + > +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()"); > -- > 1.9.1 >
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 On 22 August 2015 at 22:38, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > Hello All! > > On 2015-08-22 23:35 +0200, Yann E. MORIN spake thusly: >> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > 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. > >> --- >> package/canfestival/0003-sigval-musl.patch | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> create mode 100644 package/canfestival/0003-sigval-musl.patch >> >> diff --git a/package/canfestival/0003-sigval-musl.patch b/package/canfestival/0003-sigval-musl.patch >> new file mode 100644 >> index 0000000..7a4dadf >> --- /dev/null >> +++ b/package/canfestival/0003-sigval-musl.patch >> @@ -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" <yann.morin.1998@free.fr> >> + >> +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()"); >> -- >> 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@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Brendan, All, On 2015-08-22 22:45 +0100, Brendan Heading spake thusly: > 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. > 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. > regards > > Brendan > > > > On 22 August 2015 at 22:38, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > Hello All! > > > > On 2015-08-22 23:35 +0200, Yann E. MORIN spake thusly: > >> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > > > 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. > > > >> --- > >> package/canfestival/0003-sigval-musl.patch | 20 ++++++++++++++++++++ > >> 1 file changed, 20 insertions(+) > >> create mode 100644 package/canfestival/0003-sigval-musl.patch > >> > >> diff --git a/package/canfestival/0003-sigval-musl.patch b/package/canfestival/0003-sigval-musl.patch > >> new file mode 100644 > >> index 0000000..7a4dadf > >> --- /dev/null > >> +++ b/package/canfestival/0003-sigval-musl.patch > >> @@ -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" <yann.morin.1998@free.fr> > >> + > >> +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()"); > >> -- > >> 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@busybox.net > > http://lists.busybox.net/mailman/listinfo/buildroot
>> 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 .. > 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
>>>>> "Brendan" == Brendan Heading <brendanheading@gmail.com> writes: Hi, > 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.
Brendan, Yann, On Sat, 22 Aug 2015 23:35:00 +0100, Brendan Heading wrote: > >> 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 .. > > > 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
diff --git a/package/canfestival/0003-sigval-musl.patch b/package/canfestival/0003-sigval-musl.patch new file mode 100644 index 0000000..7a4dadf --- /dev/null +++ b/package/canfestival/0003-sigval-musl.patch @@ -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" <yann.morin.1998@free.fr> + +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()");
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> --- package/canfestival/0003-sigval-musl.patch | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 package/canfestival/0003-sigval-musl.patch