All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.