Openembedded Core Discussions
 help / color / mirror / Atom feed
* [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
@ 2019-03-12  9:41 changqing.li
  2019-03-15 10:45 ` Adrian Bunk
  0 siblings, 1 reply; 10+ messages in thread
From: changqing.li @ 2019-03-12  9:41 UTC (permalink / raw)
  To: openembedded-core

From: Changqing Li <changqing.li@windriver.com>

while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
it failed

[snip]
| Incremental.c: In function 'Incremental_container':
| Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
| close(mdfd);
| ^~~~~~~~~~~

[snip]
super-intel.c: In function 'apply_takeover_update':
| super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
| " MISSING_%d", du->index);
| ^~

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
 ...maybe-uninitialized-format-overflow-warni.patch | 60 ++++++++++++++++++++++
 meta/recipes-extended/mdadm/mdadm_4.1.bb           |  1 +
 2 files changed, 61 insertions(+)
 create mode 100644 meta/recipes-extended/mdadm/files/0001-mdadm-gcc8-maybe-uninitialized-format-overflow-warni.patch

diff --git a/meta/recipes-extended/mdadm/files/0001-mdadm-gcc8-maybe-uninitialized-format-overflow-warni.patch b/meta/recipes-extended/mdadm/files/0001-mdadm-gcc8-maybe-uninitialized-format-overflow-warni.patch
new file mode 100644
index 0000000..237f83a
--- /dev/null
+++ b/meta/recipes-extended/mdadm/files/0001-mdadm-gcc8-maybe-uninitialized-format-overflow-warni.patch
@@ -0,0 +1,60 @@
+From bf457a83834932ba06de3528b8779a023e73fa7b Mon Sep 17 00:00:00 2001
+From: Changqing Li <changqing.li@windriver.com>
+Date: Tue, 12 Mar 2019 16:17:29 +0800
+Subject: [PATCH] mdadm: gcc8 maybe-uninitialized/format-overflow warning
+
+while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
+it failed
+
+[snip]
+| Incremental.c: In function 'Incremental_container':
+| Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
+| close(mdfd);
+| ^~~~~~~~~~~
+
+[snip]
+super-intel.c: In function 'apply_takeover_update':
+| super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
+| " MISSING_%d", du->index);
+| ^~
+
+Upstream-Status: Submitted [https://github.com/neilbrown/mdadm/pull/36]
+
+Signed-off-by: Changqing Li <changqing.li@windriver.com>
+---
+ Incremental.c | 2 +-
+ super-intel.c | 4 ++--
+ 2 files changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/Incremental.c b/Incremental.c
+index a4ff7d4..b667868 100644
+--- a/Incremental.c
++++ b/Incremental.c
+@@ -1500,7 +1500,7 @@ static int Incremental_container(struct supertype *st, char *devname,
+ 		return 0;
+ 	}
+ 	for (ra = list ; ra ; ra = ra->next) {
+-		int mdfd;
++		int mdfd = 0;
+ 		char chosen_name[1024];
+ 		struct map_ent *mp;
+ 		struct mddev_ident *match = NULL;
+diff --git a/super-intel.c b/super-intel.c
+index 10d7218..c3741ea 100644
+--- a/super-intel.c
++++ b/super-intel.c
+@@ -9612,9 +9612,9 @@ static int apply_takeover_update(struct imsm_update_takeover *u,
+ 			du->major = 0;
+ 			du->index = (i * 2) + 1;
+ 			sprintf((char *)du->disk.serial,
+-				" MISSING_%d", du->index);
++				" MISSING_%hu", du->index);
+ 			sprintf((char *)du->serial,
+-				"MISSING_%d", du->index);
++				"MISSING_%hu", du->index);
+ 			du->next = super->missing;
+ 			super->missing = du;
+ 		}
+-- 
+2.7.4
+
diff --git a/meta/recipes-extended/mdadm/mdadm_4.1.bb b/meta/recipes-extended/mdadm/mdadm_4.1.bb
index 2d7ade6..d79c533 100644
--- a/meta/recipes-extended/mdadm/mdadm_4.1.bb
+++ b/meta/recipes-extended/mdadm/mdadm_4.1.bb
@@ -19,6 +19,7 @@ SRC_URI = "${KERNELORG_MIRROR}/linux/utils/raid/mdadm/${BPN}-${PV}.tar.xz \
            file://0001-fix-gcc-8-format-truncation-warning.patch \
 	   file://mdadm.init \
 	   file://mdmonitor.service \
+           file://0001-mdadm-gcc8-maybe-uninitialized-format-overflow-warni.patch \
            "
 SRC_URI[md5sum] = "51bf3651bd73a06c413a2f964f299598"
 SRC_URI[sha256sum] = "ab7688842908d3583a704d491956f31324c3a5fc9f6a04653cb75d19f1934f4a"
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
  2019-03-12  9:41 [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning changqing.li
@ 2019-03-15 10:45 ` Adrian Bunk
  2019-03-20  3:51   ` Khem Raj
  0 siblings, 1 reply; 10+ messages in thread
From: Adrian Bunk @ 2019-03-15 10:45 UTC (permalink / raw)
  To: changqing.li; +Cc: openembedded-core

On Tue, Mar 12, 2019 at 05:41:58PM +0800, changqing.li@windriver.com wrote:
> From: Changqing Li <changqing.li@windriver.com>
> 
> while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
> it failed
> 
> [snip]
> | Incremental.c: In function 'Incremental_container':
> | Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> | close(mdfd);
> | ^~~~~~~~~~~
> 
> [snip]
> super-intel.c: In function 'apply_takeover_update':
> | super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
> | " MISSING_%d", du->index);
> | ^~
>...

I am seeing these warnings only with -Og, are you also seeing them with
-Og (DEBUG_OPTIMIZATION) only?

If this is true, I would consider 
https://sources.debian.org/src/mdadm/4.1-2/debian/patches/debian-no-Werror.diff/
a better workaround.

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] 10+ messages in thread

* Re: [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
  2019-03-15 10:45 ` Adrian Bunk
@ 2019-03-20  3:51   ` Khem Raj
  2019-03-20  5:21     ` Adrian Bunk
  0 siblings, 1 reply; 10+ messages in thread
From: Khem Raj @ 2019-03-20  3:51 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Patches and discussions about the oe-core layer

On Fri, Mar 15, 2019 at 6:45 AM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Tue, Mar 12, 2019 at 05:41:58PM +0800, changqing.li@windriver.com wrote:
> > From: Changqing Li <changqing.li@windriver.com>
> >
> > while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
> > it failed
> >
> > [snip]
> > | Incremental.c: In function 'Incremental_container':
> > | Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > | close(mdfd);
> > | ^~~~~~~~~~~
> >
> > [snip]
> > super-intel.c: In function 'apply_takeover_update':
> > | super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
> > | " MISSING_%d", du->index);
> > | ^~
> >...
>
> I am seeing these warnings only with -Og, are you also seeing them with
> -Og (DEBUG_OPTIMIZATION) only?
>
> If this is true, I would consider
> https://sources.debian.org/src/mdadm/4.1-2/debian/patches/debian-no-Werror.diff/
> a better workaround.
>
This seems a broader brush, I really dont like to relegate Werror if
we dont have to, because it will force us
to fix the code. However, I am seeing it fail with clang now

https://errors.yoctoproject.org/Errors/Details/233618/
> 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
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
  2019-03-20  3:51   ` Khem Raj
@ 2019-03-20  5:21     ` Adrian Bunk
  2019-03-20 12:59       ` Khem Raj
  0 siblings, 1 reply; 10+ messages in thread
From: Adrian Bunk @ 2019-03-20  5:21 UTC (permalink / raw)
  To: Khem Raj; +Cc: Patches and discussions about the oe-core layer

On Tue, Mar 19, 2019 at 11:51:47PM -0400, Khem Raj wrote:
> On Fri, Mar 15, 2019 at 6:45 AM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > On Tue, Mar 12, 2019 at 05:41:58PM +0800, changqing.li@windriver.com wrote:
> > > From: Changqing Li <changqing.li@windriver.com>
> > >
> > > while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
> > > it failed
> > >
> > > [snip]
> > > | Incremental.c: In function 'Incremental_container':
> > > | Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > | close(mdfd);
> > > | ^~~~~~~~~~~
> > >
> > > [snip]
> > > super-intel.c: In function 'apply_takeover_update':
> > > | super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
> > > | " MISSING_%d", du->index);
> > > | ^~
> > >...
> >
> > I am seeing these warnings only with -Og, are you also seeing them with
> > -Og (DEBUG_OPTIMIZATION) only?
> >
> > If this is true, I would consider
> > https://sources.debian.org/src/mdadm/4.1-2/debian/patches/debian-no-Werror.diff/
> > a better workaround.
> >
> This seems a broader brush, I really dont like to relegate Werror if
> we dont have to, because it will force us
> to fix the code.

How are we getting such fixes properly reviewed?

What actually happens is that the easiest change that silences a warning 
gets applied.

And for such bogus -Og only warnings there was no problem in the code,
meaning regression risks by OE/Yocto-only patches without fixing anything.

> However, I am seeing it fail with clang now
> 
> https://errors.yoctoproject.org/Errors/Details/233618/

This is caused by the "fix" for the -Og gcc warning.

Which looks bogus, and might be harmful.

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] 10+ messages in thread

* Re: [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
  2019-03-20  5:21     ` Adrian Bunk
@ 2019-03-20 12:59       ` Khem Raj
  2019-03-20 13:43         ` Richard Purdie
  2019-03-20 15:00         ` Adrian Bunk
  0 siblings, 2 replies; 10+ messages in thread
From: Khem Raj @ 2019-03-20 12:59 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Patches and discussions about the oe-core layer

On Wed, Mar 20, 2019 at 1:21 AM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Tue, Mar 19, 2019 at 11:51:47PM -0400, Khem Raj wrote:
> > On Fri, Mar 15, 2019 at 6:45 AM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > On Tue, Mar 12, 2019 at 05:41:58PM +0800, changqing.li@windriver.com wrote:
> > > > From: Changqing Li <changqing.li@windriver.com>
> > > >
> > > > while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
> > > > it failed
> > > >
> > > > [snip]
> > > > | Incremental.c: In function 'Incremental_container':
> > > > | Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > | close(mdfd);
> > > > | ^~~~~~~~~~~
> > > >
> > > > [snip]
> > > > super-intel.c: In function 'apply_takeover_update':
> > > > | super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
> > > > | " MISSING_%d", du->index);
> > > > | ^~
> > > >...
> > >
> > > I am seeing these warnings only with -Og, are you also seeing them with
> > > -Og (DEBUG_OPTIMIZATION) only?
> > >
> > > If this is true, I would consider
> > > https://sources.debian.org/src/mdadm/4.1-2/debian/patches/debian-no-Werror.diff/
> > > a better workaround.
> > >
> > This seems a broader brush, I really dont like to relegate Werror if
> > we dont have to, because it will force us
> > to fix the code.
>
> How are we getting such fixes properly reviewed?
>
> What actually happens is that the easiest change that silences a warning
> gets applied.
>
> And for such bogus -Og only warnings there was no problem in the code,

so then probably a better fix is to add -Wno-error to DEBUG_FLAGS
which will limit it to -Og case, and revert the original fix.



> meaning regression risks by OE/Yocto-only patches without fixing anything.
>
> > However, I am seeing it fail with clang now
> >
> > https://errors.yoctoproject.org/Errors/Details/233618/
>
> This is caused by the "fix" for the -Og gcc warning.
>
> Which looks bogus, and might be harmful.

I think that is right. the fix seems to be regressing on gcc 9 as well
so it has to be done differently.
>
> 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] 10+ messages in thread

* Re: [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
  2019-03-20 12:59       ` Khem Raj
@ 2019-03-20 13:43         ` Richard Purdie
  2019-03-20 15:00         ` Adrian Bunk
  1 sibling, 0 replies; 10+ messages in thread
From: Richard Purdie @ 2019-03-20 13:43 UTC (permalink / raw)
  To: Khem Raj, Adrian Bunk; +Cc: Patches and discussions about the oe-core layer

On Wed, 2019-03-20 at 08:59 -0400, Khem Raj wrote:
> On Wed, Mar 20, 2019 at 1:21 AM Adrian Bunk <bunk@stusta.de> wrote:
> > On Tue, Mar 19, 2019 at 11:51:47PM -0400, Khem Raj wrote:
> > > On Fri, Mar 15, 2019 at 6:45 AM Adrian Bunk <bunk@stusta.de>
> > > wrote:
> > > > On Tue, Mar 12, 2019 at 05:41:58PM +0800, 
> > > > changqing.li@windriver.com wrote:
> > > > > From: Changqing Li <changqing.li@windriver.com>
> > > > > 
> > > > > while compiled with -Werror=maybe-uninitialized/-
> > > > > Werror=format-overflow=,
> > > > > it failed
> > > > > 
> > > > > [snip]
> > > > > > Incremental.c: In function 'Incremental_container':
> > > > > > Incremental.c:1593:3: error: 'mdfd' may be used
> > > > > > uninitialized in this function [-Werror=maybe-
> > > > > > uninitialized]
> > > > > > close(mdfd);
> > > > > > ^~~~~~~~~~~
> > > > > 
> > > > > [snip]
> > > > > super-intel.c: In function 'apply_takeover_update':
> > > > > > super-intel.c:9615:15: error: '%d' directive writing
> > > > > > between 1 and 11 bytes into a region of size 7 [-
> > > > > > Werror=format-overflow=]
> > > > > > " MISSING_%d", du->index);
> > > > > > ^~
> > > > > ...
> > > > 
> > > > I am seeing these warnings only with -Og, are you also seeing
> > > > them with
> > > > -Og (DEBUG_OPTIMIZATION) only?
> > > > 
> > > > If this is true, I would consider
> > > > https://sources.debian.org/src/mdadm/4.1-2/debian/patches/debian-no-Werror.diff/
> > > > a better workaround.
> > > > 
> > > This seems a broader brush, I really dont like to relegate Werror
> > > if
> > > we dont have to, because it will force us
> > > to fix the code.
> > 
> > How are we getting such fixes properly reviewed?
> > 
> > What actually happens is that the easiest change that silences a
> > warning
> > gets applied.
> > 
> > And for such bogus -Og only warnings there was no problem in the
> > code,
> 
> so then probably a better fix is to add -Wno-error to DEBUG_FLAGS
> which will limit it to -Og case, and revert the original fix.
> 
> 
> 
> > meaning regression risks by OE/Yocto-only patches without fixing
> > anything.
> > 
> > > However, I am seeing it fail with clang now
> > > 
> > > https://errors.yoctoproject.org/Errors/Details/233618/
> > 
> > This is caused by the "fix" for the -Og gcc warning.
> > 
> > Which looks bogus, and might be harmful.
> 
> I think that is right. the fix seems to be regressing on gcc 9 as
> well so it has to be done differently.

Agreed, we should revert it, its creating more problems than it solves.

Cheers,

Richard




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
  2019-03-20 12:59       ` Khem Raj
  2019-03-20 13:43         ` Richard Purdie
@ 2019-03-20 15:00         ` Adrian Bunk
  2019-03-20 17:06           ` Khem Raj
  1 sibling, 1 reply; 10+ messages in thread
From: Adrian Bunk @ 2019-03-20 15:00 UTC (permalink / raw)
  To: Khem Raj; +Cc: Patches and discussions about the oe-core layer

On Wed, Mar 20, 2019 at 08:59:39AM -0400, Khem Raj wrote:
> On Wed, Mar 20, 2019 at 1:21 AM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > On Tue, Mar 19, 2019 at 11:51:47PM -0400, Khem Raj wrote:
> > > On Fri, Mar 15, 2019 at 6:45 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > >
> > > > On Tue, Mar 12, 2019 at 05:41:58PM +0800, changqing.li@windriver.com wrote:
> > > > > From: Changqing Li <changqing.li@windriver.com>
> > > > >
> > > > > while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
> > > > > it failed
> > > > >
> > > > > [snip]
> > > > > | Incremental.c: In function 'Incremental_container':
> > > > > | Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > > | close(mdfd);
> > > > > | ^~~~~~~~~~~
> > > > >
> > > > > [snip]
> > > > > super-intel.c: In function 'apply_takeover_update':
> > > > > | super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
> > > > > | " MISSING_%d", du->index);
> > > > > | ^~
> > > > >...
> > > >
> > > > I am seeing these warnings only with -Og, are you also seeing them with
> > > > -Og (DEBUG_OPTIMIZATION) only?
> > > >
> > > > If this is true, I would consider
> > > > https://sources.debian.org/src/mdadm/4.1-2/debian/patches/debian-no-Werror.diff/
> > > > a better workaround.
> > > >
> > > This seems a broader brush, I really dont like to relegate Werror if
> > > we dont have to, because it will force us
> > > to fix the code.
> >
> > How are we getting such fixes properly reviewed?
> >
> > What actually happens is that the easiest change that silences a warning
> > gets applied.
> >
> > And for such bogus -Og only warnings there was no problem in the code,
> 
> so then probably a better fix is to add -Wno-error to DEBUG_FLAGS
> which will limit it to -Og case, and revert the original fix.

For mdadm this might work, for puzzles it would not.

Order matters, and upstream can insert your flags before or after
the -Werror. So unfortunately this would not cover all recipes.

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] 10+ messages in thread

* Re: [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
  2019-03-20 15:00         ` Adrian Bunk
@ 2019-03-20 17:06           ` Khem Raj
  2019-03-22  3:49             ` Changqing Li
  2019-03-22  3:57             ` [PATCH V2] mdadm: add -Wno-error to DEBUG_OPTIMIZATION changqing.li
  0 siblings, 2 replies; 10+ messages in thread
From: Khem Raj @ 2019-03-20 17:06 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Patches and discussions about the oe-core layer

On Wed, Mar 20, 2019 at 11:00 AM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Wed, Mar 20, 2019 at 08:59:39AM -0400, Khem Raj wrote:
> > On Wed, Mar 20, 2019 at 1:21 AM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > On Tue, Mar 19, 2019 at 11:51:47PM -0400, Khem Raj wrote:
> > > > On Fri, Mar 15, 2019 at 6:45 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > > >
> > > > > On Tue, Mar 12, 2019 at 05:41:58PM +0800, changqing.li@windriver.com wrote:
> > > > > > From: Changqing Li <changqing.li@windriver.com>
> > > > > >
> > > > > > while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
> > > > > > it failed
> > > > > >
> > > > > > [snip]
> > > > > > | Incremental.c: In function 'Incremental_container':
> > > > > > | Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > > > | close(mdfd);
> > > > > > | ^~~~~~~~~~~
> > > > > >
> > > > > > [snip]
> > > > > > super-intel.c: In function 'apply_takeover_update':
> > > > > > | super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
> > > > > > | " MISSING_%d", du->index);
> > > > > > | ^~
> > > > > >...
> > > > >
> > > > > I am seeing these warnings only with -Og, are you also seeing them with
> > > > > -Og (DEBUG_OPTIMIZATION) only?
> > > > >
> > > > > If this is true, I would consider
> > > > > https://sources.debian.org/src/mdadm/4.1-2/debian/patches/debian-no-Werror.diff/
> > > > > a better workaround.
> > > > >
> > > > This seems a broader brush, I really dont like to relegate Werror if
> > > > we dont have to, because it will force us
> > > > to fix the code.
> > >
> > > How are we getting such fixes properly reviewed?
> > >
> > > What actually happens is that the easiest change that silences a warning
> > > gets applied.
> > >
> > > And for such bogus -Og only warnings there was no problem in the code,
> >
> > so then probably a better fix is to add -Wno-error to DEBUG_FLAGS
> > which will limit it to -Og case, and revert the original fix.
>
> For mdadm this might work, for puzzles it would not.
>
> Order matters, and upstream can insert your flags before or after
> the -Werror. So unfortunately this would not cover all recipes.
>

suggestion was not global, but just for this recipe. I still would
suggest that we disable it only where needed

> 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] 10+ messages in thread

* Re: [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning
  2019-03-20 17:06           ` Khem Raj
@ 2019-03-22  3:49             ` Changqing Li
  2019-03-22  3:57             ` [PATCH V2] mdadm: add -Wno-error to DEBUG_OPTIMIZATION changqing.li
  1 sibling, 0 replies; 10+ messages in thread
From: Changqing Li @ 2019-03-22  3:49 UTC (permalink / raw)
  To: Khem Raj, Adrian Bunk; +Cc: Patches and discussions about the oe-core layer


On 3/21/19 1:06 AM, Khem Raj wrote:
> On Wed, Mar 20, 2019 at 11:00 AM Adrian Bunk <bunk@stusta.de> wrote:
>> On Wed, Mar 20, 2019 at 08:59:39AM -0400, Khem Raj wrote:
>>> On Wed, Mar 20, 2019 at 1:21 AM Adrian Bunk <bunk@stusta.de> wrote:
>>>> On Tue, Mar 19, 2019 at 11:51:47PM -0400, Khem Raj wrote:
>>>>> On Fri, Mar 15, 2019 at 6:45 AM Adrian Bunk <bunk@stusta.de> wrote:
>>>>>> On Tue, Mar 12, 2019 at 05:41:58PM +0800, changqing.li@windriver.com wrote:
>>>>>>> From: Changqing Li <changqing.li@windriver.com>
>>>>>>>
>>>>>>> while compiled with -Werror=maybe-uninitialized/-Werror=format-overflow=,
>>>>>>> it failed
>>>>>>>
>>>>>>> [snip]
>>>>>>> | Incremental.c: In function 'Incremental_container':
>>>>>>> | Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>>>>>> | close(mdfd);
>>>>>>> | ^~~~~~~~~~~
>>>>>>>
>>>>>>> [snip]
>>>>>>> super-intel.c: In function 'apply_takeover_update':
>>>>>>> | super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
>>>>>>> | " MISSING_%d", du->index);
>>>>>>> | ^~
>>>>>>> ...
>>>>>> I am seeing these warnings only with -Og, are you also seeing them with
>>>>>> -Og (DEBUG_OPTIMIZATION) only?
yes.
>>>>>>
>>>>>> If this is true, I would consider
>>>>>> https://sources.debian.org/src/mdadm/4.1-2/debian/patches/debian-no-Werror.diff/
>>>>>> a better workaround.
>>>>>>
>>>>> This seems a broader brush, I really dont like to relegate Werror if
>>>>> we dont have to, because it will force us
>>>>> to fix the code.
>>>> How are we getting such fixes properly reviewed?
>>>>
>>>> What actually happens is that the easiest change that silences a warning
>>>> gets applied.
>>>>
>>>> And for such bogus -Og only warnings there was no problem in the code,
>>> so then probably a better fix is to add -Wno-error to DEBUG_FLAGS
>>> which will limit it to -Og case, and revert the original fix.

Both DEBUG_OPTIMIZATION and FULL_OPTIMIZATION appended DEBUG_FLAGS.

in order limit it to -Og case,  I will add  -Wno-error to 
DEBUG_OPTIMIZATION, and send a v2.


[snip]

FULL_OPTIMIZATION = "-O2 -pipe ${DEBUG_FLAGS}"

DEBUG_OPTIMIZATION = "-Og ${DEBUG_FLAGS} -pipe"

SELECTED_OPTIMIZATION = "${@d.getVar(oe.utils.vartrue('DEBUG_BUILD', 
'DEBUG_OPTIMIZATION', 'FULL_OPTIMIZATION', d))}"




>> For mdadm this might work, for puzzles it would not.
>>
>> Order matters, and upstream can insert your flags before or after
>> the -Werror. So unfortunately this would not cover all recipes.
>>
> suggestion was not global, but just for this recipe. I still would
> suggest that we disable it only where needed
>
>> 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
>>
-- 
BRs

Sandy(Li Changqing)



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V2] mdadm: add -Wno-error to DEBUG_OPTIMIZATION
  2019-03-20 17:06           ` Khem Raj
  2019-03-22  3:49             ` Changqing Li
@ 2019-03-22  3:57             ` changqing.li
  1 sibling, 0 replies; 10+ messages in thread
From: changqing.li @ 2019-03-22  3:57 UTC (permalink / raw)
  To: openembedded-core

From: Changqing Li <changqing.li@windriver.com>

when compile with DEBUG_OPTIMIZATION(-Og), compile failed with below
error, fix by add -Wno-error:

[snip]
| Incremental.c: In function 'Incremental_container':
| Incremental.c:1593:3: error: 'mdfd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
| close(mdfd);
| ^~~~~~~~~~~

[snip]
super-intel.c: In function 'apply_takeover_update':
| super-intel.c:9615:15: error: '%d' directive writing between 1 and 11 bytes into a region of size 7 [-Werror=format-overflow=]
| " MISSING_%d", du->index);
| ^~
...

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
 meta/recipes-extended/mdadm/mdadm_4.1.bb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/recipes-extended/mdadm/mdadm_4.1.bb b/meta/recipes-extended/mdadm/mdadm_4.1.bb
index d79c533..ab3ea41 100644
--- a/meta/recipes-extended/mdadm/mdadm_4.1.bb
+++ b/meta/recipes-extended/mdadm/mdadm_4.1.bb
@@ -41,6 +41,8 @@ CFLAGS_append_mipsarchn32 = ' -D__SANE_USERSPACE_TYPES__'
 
 EXTRA_OEMAKE = 'CHECK_RUN_DIR=0 CXFLAGS="${CFLAGS}"'
 
+DEBUG_OPTIMIZATION_append = " -Wno-error"
+
 do_compile() {
 	# Point to right sbindir
 	sed -i -e "s;BINDIR  = /sbin;BINDIR = $base_sbindir;" -e "s;UDEVDIR = /lib;UDEVDIR = $nonarch_base_libdir;" ${S}/Makefile
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-03-22  3:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-12  9:41 [PATCH] mdadm: fix gcc8 maybe-uninitialized/format-overflow warning changqing.li
2019-03-15 10:45 ` Adrian Bunk
2019-03-20  3:51   ` Khem Raj
2019-03-20  5:21     ` Adrian Bunk
2019-03-20 12:59       ` Khem Raj
2019-03-20 13:43         ` Richard Purdie
2019-03-20 15:00         ` Adrian Bunk
2019-03-20 17:06           ` Khem Raj
2019-03-22  3:49             ` Changqing Li
2019-03-22  3:57             ` [PATCH V2] mdadm: add -Wno-error to DEBUG_OPTIMIZATION changqing.li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox