* [meta-oe][PATCH] libwebsockets: Fix the build with -Os
@ 2019-08-29 10:39 Adrian Bunk
2019-08-29 18:46 ` Khem Raj
0 siblings, 1 reply; 9+ messages in thread
From: Adrian Bunk @ 2019-08-29 10:39 UTC (permalink / raw)
To: openembedded-devel
lib/event-libs/libuv/libuv.c: In function 'elops_destroy_context1_uv':
lib/event-libs/libuv/libuv.c:519:7: error: 'm' may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (m)
^
Signed-off-by: Adrian Bunk <bunk@stusta.de>
---
.../recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb | 2 ++
1 file changed, 2 insertions(+)
diff --git a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
index 50620d99e..fcabeb902 100644
--- a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
+++ b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
@@ -28,3 +28,5 @@ EXTRA_OECMAKE += " \
PACKAGES =+ "${PN}-testapps"
FILES_${PN}-testapps += "${datadir}/libwebsockets-test-server/*"
+
+CFLAGS_append = " -Wno-error"
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] libwebsockets: Fix the build with -Os
2019-08-29 10:39 [meta-oe][PATCH] libwebsockets: Fix the build with -Os Adrian Bunk
@ 2019-08-29 18:46 ` Khem Raj
2019-08-29 19:14 ` Adrian Bunk
0 siblings, 1 reply; 9+ messages in thread
From: Khem Raj @ 2019-08-29 18:46 UTC (permalink / raw)
To: Adrian Bunk; +Cc: openembeded-devel
On Thu, Aug 29, 2019 at 3:39 AM Adrian Bunk <bunk@stusta.de> wrote:
>
> lib/event-libs/libuv/libuv.c: In function 'elops_destroy_context1_uv':
> lib/event-libs/libuv/libuv.c:519:7: error: 'm' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> if (m)
> ^
>
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> ---
> .../recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> index 50620d99e..fcabeb902 100644
> --- a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> +++ b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> @@ -28,3 +28,5 @@ EXTRA_OECMAKE += " \
> PACKAGES =+ "${PN}-testapps"
>
> FILES_${PN}-testapps += "${datadir}/libwebsockets-test-server/*"
> +
> +CFLAGS_append = " -Wno-error"
is it possible to fix the underlying problem? since Os is not default
it definitely could be a bug in upstream but
by disabling warnings for all kind of builds we are painting with broad brush
> --
> 2.17.1
>
> --
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] libwebsockets: Fix the build with -Os
2019-08-29 18:46 ` Khem Raj
@ 2019-08-29 19:14 ` Adrian Bunk
2019-08-29 19:54 ` Khem Raj
0 siblings, 1 reply; 9+ messages in thread
From: Adrian Bunk @ 2019-08-29 19:14 UTC (permalink / raw)
To: Khem Raj; +Cc: openembeded-devel
On Thu, Aug 29, 2019 at 11:46:55AM -0700, Khem Raj wrote:
> On Thu, Aug 29, 2019 at 3:39 AM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > lib/event-libs/libuv/libuv.c: In function 'elops_destroy_context1_uv':
> > lib/event-libs/libuv/libuv.c:519:7: error: 'm' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > if (m)
> > ^
> >
> > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > ---
> > .../recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > index 50620d99e..fcabeb902 100644
> > --- a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > +++ b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > @@ -28,3 +28,5 @@ EXTRA_OECMAKE += " \
> > PACKAGES =+ "${PN}-testapps"
> >
> > FILES_${PN}-testapps += "${datadir}/libwebsockets-test-server/*"
> > +
> > +CFLAGS_append = " -Wno-error"
>
> is it possible to fix the underlying problem? since Os is not default
> it definitely could be a bug in upstream but
> by disabling warnings for all kind of builds we are painting with broad brush
The underlying problem is that some gcc warnings are not reliable with -Os,
there are bugs open in the gcc bugzilla for that.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] libwebsockets: Fix the build with -Os
2019-08-29 19:14 ` Adrian Bunk
@ 2019-08-29 19:54 ` Khem Raj
2019-08-29 20:25 ` Adrian Bunk
0 siblings, 1 reply; 9+ messages in thread
From: Khem Raj @ 2019-08-29 19:54 UTC (permalink / raw)
To: Adrian Bunk; +Cc: openembeded-devel
On Thu, Aug 29, 2019 at 12:14 PM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Thu, Aug 29, 2019 at 11:46:55AM -0700, Khem Raj wrote:
> > On Thu, Aug 29, 2019 at 3:39 AM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > lib/event-libs/libuv/libuv.c: In function 'elops_destroy_context1_uv':
> > > lib/event-libs/libuv/libuv.c:519:7: error: 'm' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > if (m)
> > > ^
> > >
> > > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > > ---
> > > .../recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > index 50620d99e..fcabeb902 100644
> > > --- a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > +++ b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > @@ -28,3 +28,5 @@ EXTRA_OECMAKE += " \
> > > PACKAGES =+ "${PN}-testapps"
> > >
> > > FILES_${PN}-testapps += "${datadir}/libwebsockets-test-server/*"
> > > +
> > > +CFLAGS_append = " -Wno-error"
> >
> > is it possible to fix the underlying problem? since Os is not default
> > it definitely could be a bug in upstream but
> > by disabling warnings for all kind of builds we are painting with broad brush
>
> The underlying problem is that some gcc warnings are not reliable with -Os,
> there are bugs open in the gcc bugzilla for that.
>
I am aware of that for maybe-* warnings heuristics may go wrong, but
then its better to just disable that
one warning from being treated as error if thats possible to add
easily something like
-Wno-error=maybe-uninitialized could do it.
> cu
> Adrian
>
> --
>
> "Is there not promise of rain?" Ling Tan asked suddenly out
> of the darkness. There had been need of rain for many days.
> "Only a promise," Lao Er said.
> Pearl S. Buck - Dragon Seed
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] libwebsockets: Fix the build with -Os
2019-08-29 19:54 ` Khem Raj
@ 2019-08-29 20:25 ` Adrian Bunk
2019-08-29 20:51 ` Khem Raj
0 siblings, 1 reply; 9+ messages in thread
From: Adrian Bunk @ 2019-08-29 20:25 UTC (permalink / raw)
To: Khem Raj; +Cc: openembeded-devel
On Thu, Aug 29, 2019 at 12:54:16PM -0700, Khem Raj wrote:
> On Thu, Aug 29, 2019 at 12:14 PM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > On Thu, Aug 29, 2019 at 11:46:55AM -0700, Khem Raj wrote:
> > > On Thu, Aug 29, 2019 at 3:39 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > >
> > > > lib/event-libs/libuv/libuv.c: In function 'elops_destroy_context1_uv':
> > > > lib/event-libs/libuv/libuv.c:519:7: error: 'm' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > if (m)
> > > > ^
> > > >
> > > > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > > > ---
> > > > .../recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > index 50620d99e..fcabeb902 100644
> > > > --- a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > +++ b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > @@ -28,3 +28,5 @@ EXTRA_OECMAKE += " \
> > > > PACKAGES =+ "${PN}-testapps"
> > > >
> > > > FILES_${PN}-testapps += "${datadir}/libwebsockets-test-server/*"
> > > > +
> > > > +CFLAGS_append = " -Wno-error"
> > >
> > > is it possible to fix the underlying problem? since Os is not default
> > > it definitely could be a bug in upstream but
> > > by disabling warnings for all kind of builds we are painting with broad brush
> >
> > The underlying problem is that some gcc warnings are not reliable with -Os,
> > there are bugs open in the gcc bugzilla for that.
> >
> I am aware of that for maybe-* warnings heuristics may go wrong, but
> then its better to just disable that
> one warning from being treated as error if thats possible to add
> easily something like
> -Wno-error=maybe-uninitialized could do it.
And then the package fails to build due to a different warning after the
next gcc upgrade.
The few packages that manually set -Werror are causing so much trouble,
and seeing warnings as errors in this code from 2018 that is currently
298 commits behind upstream master won't bring actual benefits even
when the warnings are not gcc bugs.
If you are interested in warnings you shouldn't have -Werror in very few
packages, but check for the most problematic warnings in all packages.
E.g. -Wimplicit-function-declaration warnings are often the cause for
runime crashes, and a quick grep through my build logs shows that your
gettid patches to snort/lttng-tools/lttng-ust are not correct.[1]
cu
Adrian
[1] pid_t is an int in glibc, which makes the lack of prototypes
harmless in this specific case.
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] libwebsockets: Fix the build with -Os
2019-08-29 20:25 ` Adrian Bunk
@ 2019-08-29 20:51 ` Khem Raj
2019-08-29 21:23 ` Adrian Bunk
0 siblings, 1 reply; 9+ messages in thread
From: Khem Raj @ 2019-08-29 20:51 UTC (permalink / raw)
To: Adrian Bunk; +Cc: openembeded-devel
On Thu, Aug 29, 2019 at 1:25 PM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Thu, Aug 29, 2019 at 12:54:16PM -0700, Khem Raj wrote:
> > On Thu, Aug 29, 2019 at 12:14 PM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > On Thu, Aug 29, 2019 at 11:46:55AM -0700, Khem Raj wrote:
> > > > On Thu, Aug 29, 2019 at 3:39 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > > >
> > > > > lib/event-libs/libuv/libuv.c: In function 'elops_destroy_context1_uv':
> > > > > lib/event-libs/libuv/libuv.c:519:7: error: 'm' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > > if (m)
> > > > > ^
> > > > >
> > > > > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > > > > ---
> > > > > .../recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > index 50620d99e..fcabeb902 100644
> > > > > --- a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > +++ b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > @@ -28,3 +28,5 @@ EXTRA_OECMAKE += " \
> > > > > PACKAGES =+ "${PN}-testapps"
> > > > >
> > > > > FILES_${PN}-testapps += "${datadir}/libwebsockets-test-server/*"
> > > > > +
> > > > > +CFLAGS_append = " -Wno-error"
> > > >
> > > > is it possible to fix the underlying problem? since Os is not default
> > > > it definitely could be a bug in upstream but
> > > > by disabling warnings for all kind of builds we are painting with broad brush
> > >
> > > The underlying problem is that some gcc warnings are not reliable with -Os,
> > > there are bugs open in the gcc bugzilla for that.
> > >
> > I am aware of that for maybe-* warnings heuristics may go wrong, but
> > then its better to just disable that
> > one warning from being treated as error if thats possible to add
> > easily something like
> > -Wno-error=maybe-uninitialized could do it.
>
> And then the package fails to build due to a different warning after the
> next gcc upgrade.
>
which is fine, since we can report it upstream to either gcc if its
gcc's fault or to package itself
and better still with a patch. Ideally we should use package defaults
or maybe be more strict
If everyone stop using these warnings then why is compiler adding
them in the first place
here I realize lies the difference of opinion.
> The few packages that manually set -Werror are causing so much trouble,
> and seeing warnings as errors in this code from 2018 that is currently
> 298 commits behind upstream master won't bring actual benefits even
> when the warnings are not gcc bugs.
>
> If you are interested in warnings you shouldn't have -Werror in very few
> packages, but check for the most problematic warnings in all packages.
>
> E.g. -Wimplicit-function-declaration warnings are often the cause for
> runime crashes, and a quick grep through my build logs shows that your
> gettid patches to snort/lttng-tools/lttng-ust are not correct.[1]
>
maybe you should explain a bit more why they not correct
these patches are to define local gettid function if system libc does
not provide it
there is a new syscall wrapper in glibc 2.30, older glibcs wont have it.
> cu
> Adrian
>
> [1] pid_t is an int in glibc, which makes the lack of prototypes
> harmless in this specific case.
>
> --
>
> "Is there not promise of rain?" Ling Tan asked suddenly out
> of the darkness. There had been need of rain for many days.
> "Only a promise," Lao Er said.
> Pearl S. Buck - Dragon Seed
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] libwebsockets: Fix the build with -Os
2019-08-29 20:51 ` Khem Raj
@ 2019-08-29 21:23 ` Adrian Bunk
2019-08-29 22:09 ` Khem Raj
0 siblings, 1 reply; 9+ messages in thread
From: Adrian Bunk @ 2019-08-29 21:23 UTC (permalink / raw)
To: Khem Raj; +Cc: openembeded-devel
On Thu, Aug 29, 2019 at 01:51:05PM -0700, Khem Raj wrote:
> On Thu, Aug 29, 2019 at 1:25 PM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > On Thu, Aug 29, 2019 at 12:54:16PM -0700, Khem Raj wrote:
> > > On Thu, Aug 29, 2019 at 12:14 PM Adrian Bunk <bunk@stusta.de> wrote:
> > > >
> > > > On Thu, Aug 29, 2019 at 11:46:55AM -0700, Khem Raj wrote:
> > > > > On Thu, Aug 29, 2019 at 3:39 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > > > >
> > > > > > lib/event-libs/libuv/libuv.c: In function 'elops_destroy_context1_uv':
> > > > > > lib/event-libs/libuv/libuv.c:519:7: error: 'm' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > > > if (m)
> > > > > > ^
> > > > > >
> > > > > > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > > > > > ---
> > > > > > .../recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb | 2 ++
> > > > > > 1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > > index 50620d99e..fcabeb902 100644
> > > > > > --- a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > > +++ b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > > @@ -28,3 +28,5 @@ EXTRA_OECMAKE += " \
> > > > > > PACKAGES =+ "${PN}-testapps"
> > > > > >
> > > > > > FILES_${PN}-testapps += "${datadir}/libwebsockets-test-server/*"
> > > > > > +
> > > > > > +CFLAGS_append = " -Wno-error"
> > > > >
> > > > > is it possible to fix the underlying problem? since Os is not default
> > > > > it definitely could be a bug in upstream but
> > > > > by disabling warnings for all kind of builds we are painting with broad brush
> > > >
> > > > The underlying problem is that some gcc warnings are not reliable with -Os,
> > > > there are bugs open in the gcc bugzilla for that.
> > > >
> > > I am aware of that for maybe-* warnings heuristics may go wrong, but
> > > then its better to just disable that
> > > one warning from being treated as error if thats possible to add
> > > easily something like
> > > -Wno-error=maybe-uninitialized could do it.
> >
> > And then the package fails to build due to a different warning after the
> > next gcc upgrade.
> >
>
> which is fine, since we can report it upstream to either gcc if its
> gcc's fault or to package itself
This is often a waste of time since the code in OE is often much older
than upstream master.
> and better still with a patch.
But please do not add such patches to OE.
Patches from people who don't know the code well are often quite buggy,
and fixing warnings then tends to add more bugs than it fixes.
Google "Debian OpenSSL disaster" for how the Debian maintainer "fixing"
a Valgrind warning in the Debian OpenSSL package made private keys used
for ssh authentication in Debian/Ubuntu predictable (AKA everyone on the
internet could log into the affected machines).
> Ideally we should use package defaults
> or maybe be more strict
> If everyone stop using these warnings then why is compiler adding
> them in the first place
> here I realize lies the difference of opinion.
When you are developing software these warnings are very useful.
A distribution is not developing software, it is distributing
software developed by other people.
> > The few packages that manually set -Werror are causing so much trouble,
> > and seeing warnings as errors in this code from 2018 that is currently
> > 298 commits behind upstream master won't bring actual benefits even
> > when the warnings are not gcc bugs.
> >
> > If you are interested in warnings you shouldn't have -Werror in very few
> > packages, but check for the most problematic warnings in all packages.
> >
> > E.g. -Wimplicit-function-declaration warnings are often the cause for
> > runime crashes, and a quick grep through my build logs shows that your
> > gettid patches to snort/lttng-tools/lttng-ust are not correct.[1]
>
> maybe you should explain a bit more why they not correct
> these patches are to define local gettid function if system libc does
> not provide it
> there is a new syscall wrapper in glibc 2.30, older glibcs wont have it.
...
../../../../lttng-tools-2.10.7/src/common/error.h:154:27: warning: implicit declaration of function 'getpid'; did you mean 'getpt'? [-Wimplicit-function-declaration]
../../../../lttng-tools-2.10.7/src/common/error.h:154:44: warning: implicit declaration of function 'gettid'; did you mean 'getcpu'? [-Wimplicit-function-declaration]
...
You ifdef'ed away the #include <unistd.h> that provides the prototypes
for these functions in glibc 2.30.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] libwebsockets: Fix the build with -Os
2019-08-29 21:23 ` Adrian Bunk
@ 2019-08-29 22:09 ` Khem Raj
2019-08-30 5:13 ` Adrian Bunk
0 siblings, 1 reply; 9+ messages in thread
From: Khem Raj @ 2019-08-29 22:09 UTC (permalink / raw)
To: Adrian Bunk; +Cc: openembeded-devel
On Thu, Aug 29, 2019 at 2:23 PM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Thu, Aug 29, 2019 at 01:51:05PM -0700, Khem Raj wrote:
> > On Thu, Aug 29, 2019 at 1:25 PM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > On Thu, Aug 29, 2019 at 12:54:16PM -0700, Khem Raj wrote:
> > > > On Thu, Aug 29, 2019 at 12:14 PM Adrian Bunk <bunk@stusta.de> wrote:
> > > > >
> > > > > On Thu, Aug 29, 2019 at 11:46:55AM -0700, Khem Raj wrote:
> > > > > > On Thu, Aug 29, 2019 at 3:39 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > > > > >
> > > > > > > lib/event-libs/libuv/libuv.c: In function 'elops_destroy_context1_uv':
> > > > > > > lib/event-libs/libuv/libuv.c:519:7: error: 'm' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > > > > if (m)
> > > > > > > ^
> > > > > > >
> > > > > > > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > > > > > > ---
> > > > > > > .../recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb | 2 ++
> > > > > > > 1 file changed, 2 insertions(+)
> > > > > > >
> > > > > > > diff --git a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > > > index 50620d99e..fcabeb902 100644
> > > > > > > --- a/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > > > +++ b/meta-oe/recipes-connectivity/libwebsockets/libwebsockets_3.1.0.bb
> > > > > > > @@ -28,3 +28,5 @@ EXTRA_OECMAKE += " \
> > > > > > > PACKAGES =+ "${PN}-testapps"
> > > > > > >
> > > > > > > FILES_${PN}-testapps += "${datadir}/libwebsockets-test-server/*"
> > > > > > > +
> > > > > > > +CFLAGS_append = " -Wno-error"
> > > > > >
> > > > > > is it possible to fix the underlying problem? since Os is not default
> > > > > > it definitely could be a bug in upstream but
> > > > > > by disabling warnings for all kind of builds we are painting with broad brush
> > > > >
> > > > > The underlying problem is that some gcc warnings are not reliable with -Os,
> > > > > there are bugs open in the gcc bugzilla for that.
> > > > >
> > > > I am aware of that for maybe-* warnings heuristics may go wrong, but
> > > > then its better to just disable that
> > > > one warning from being treated as error if thats possible to add
> > > > easily something like
> > > > -Wno-error=maybe-uninitialized could do it.
> > >
> > > And then the package fails to build due to a different warning after the
> > > next gcc upgrade.
> > >
> >
> > which is fine, since we can report it upstream to either gcc if its
> > gcc's fault or to package itself
>
> This is often a waste of time since the code in OE is often much older
> than upstream master.
>
which is partly because we dont have conducive workflow and tooling
for devs to upstream the patches or
consider building master branch of upstream repos easily. Maybe some
thought for future enhancements where
we can have a develop mode for packages which we can use for doing
this kind of work.
> > and better still with a patch.
>
> But please do not add such patches to OE.
>
> Patches from people who don't know the code well are often quite buggy,
> and fixing warnings then tends to add more bugs than it fixes.
>
> Google "Debian OpenSSL disaster" for how the Debian maintainer "fixing"
> a Valgrind warning in the Debian OpenSSL package made private keys used
> for ssh authentication in Debian/Ubuntu predictable (AKA everyone on the
> internet could log into the affected machines).
>
right I remember that, but then I also know first-hand cases where the
compiler was telling you all the way and it was
ignored which ended up in field bugs, so there is no right answer.
> > Ideally we should use package defaults
> > or maybe be more strict
> > If everyone stop using these warnings then why is compiler adding
> > them in the first place
> > here I realize lies the difference of opinion.
>
> When you are developing software these warnings are very useful.
>
> A distribution is not developing software, it is distributing
> software developed by other people.
>
we also provide development environment along with distro building
tools so its in interest for us to enable
not only packagers but also package developers
> > > The few packages that manually set -Werror are causing so much trouble,
> > > and seeing warnings as errors in this code from 2018 that is currently
> > > 298 commits behind upstream master won't bring actual benefits even
> > > when the warnings are not gcc bugs.
> > >
> > > If you are interested in warnings you shouldn't have -Werror in very few
> > > packages, but check for the most problematic warnings in all packages.
> > >
> > > E.g. -Wimplicit-function-declaration warnings are often the cause for
> > > runime crashes, and a quick grep through my build logs shows that your
> > > gettid patches to snort/lttng-tools/lttng-ust are not correct.[1]
> >
> > maybe you should explain a bit more why they not correct
> > these patches are to define local gettid function if system libc does
> > not provide it
> > there is a new syscall wrapper in glibc 2.30, older glibcs wont have it.
>
> ...
> ../../../../lttng-tools-2.10.7/src/common/error.h:154:27: warning: implicit declaration of function 'getpid'; did you mean 'getpt'? [-Wimplicit-function-declaration]
> ../../../../lttng-tools-2.10.7/src/common/error.h:154:44: warning: implicit declaration of function 'gettid'; did you mean 'getcpu'? [-Wimplicit-function-declaration]
> ...
>
> You ifdef'ed away the #include <unistd.h> that provides the prototypes
> for these functions in glibc 2.30.
>
> cu
> Adrian
>
> --
>
> "Is there not promise of rain?" Ling Tan asked suddenly out
> of the darkness. There had been need of rain for many days.
> "Only a promise," Lao Er said.
> Pearl S. Buck - Dragon Seed
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [meta-oe][PATCH] libwebsockets: Fix the build with -Os
2019-08-29 22:09 ` Khem Raj
@ 2019-08-30 5:13 ` Adrian Bunk
0 siblings, 0 replies; 9+ messages in thread
From: Adrian Bunk @ 2019-08-30 5:13 UTC (permalink / raw)
To: Khem Raj; +Cc: openembeded-devel
On Thu, Aug 29, 2019 at 03:09:17PM -0700, Khem Raj wrote:
> On Thu, Aug 29, 2019 at 2:23 PM Adrian Bunk <bunk@stusta.de> wrote:
> > On Thu, Aug 29, 2019 at 01:51:05PM -0700, Khem Raj wrote:
>...
> > > and better still with a patch.
> >
> > But please do not add such patches to OE.
> >
> > Patches from people who don't know the code well are often quite buggy,
> > and fixing warnings then tends to add more bugs than it fixes.
> >
> > Google "Debian OpenSSL disaster" for how the Debian maintainer "fixing"
> > a Valgrind warning in the Debian OpenSSL package made private keys used
> > for ssh authentication in Debian/Ubuntu predictable (AKA everyone on the
> > internet could log into the affected machines).
>
> right I remember that, but then I also know first-hand cases where the
> compiler was telling you all the way and it was
> ignored which ended up in field bugs, so there is no right answer.
>...
That's a lesson for upstream, not so much for a distribution.
The worst case is when people are just doing whatever is the fastest
code "fix" to silence a warning/error.
When the compiler is telling that the C library does not support
FNM_EXTMATCH, then ignoring the error with
#define FNM_EXTMATCH 0
can turn it into a field bug.
Ignoring the compile error when the C library does not support qsort_r
by using qsort instead can create exactly the runtime race conditions
qsort_r is designed to avoid.
Finding correct solutions can be hard and time-consuming,
especially when the person doing the change does not know
the code in question well.
But few correct fixes are better than many quick fixes that might
introduce more bugs than they fix.
And there is also a blame game involved:
If upstream software contains bugs, the blame goes to upstream.
If distribution patches introduce bugs, the blame goes to the
distribution.
Heartbleed was even worse than the above mentioned bug, but noone
could blame Debian for it.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-30 5:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-29 10:39 [meta-oe][PATCH] libwebsockets: Fix the build with -Os Adrian Bunk
2019-08-29 18:46 ` Khem Raj
2019-08-29 19:14 ` Adrian Bunk
2019-08-29 19:54 ` Khem Raj
2019-08-29 20:25 ` Adrian Bunk
2019-08-29 20:51 ` Khem Raj
2019-08-29 21:23 ` Adrian Bunk
2019-08-29 22:09 ` Khem Raj
2019-08-30 5:13 ` Adrian Bunk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.