[PATCH 00/28] Reenable maybe-uninitialized warnings

[PATCH 00/28] Reenable maybe-uninitialized warnings

[Arnd Bergmann]

This is a set of patches that I hope to get into v4.9 in some form
in order to turn on the -Wmaybe-uninitialized warnings again.

After talking to Linus in person at Linaro Connect about this, I
spent some time on finding all the remaining warnings, and this
is the resulting patch series. More details are in the description
of the last patch that actually enables the warning.

Let me know if there are other warnings that I missed, and whether
you think these are still appropriate for v4.9 or not.
A couple of patches are non-obvious, and could use some more
detailed review.

	Arnd

Arnd Bergmann (28):
  [v2] netfilter: nf_tables: avoid uninitialized variable warning
  [v2] mtd: mtk: avoid warning in mtk_ecc_encode
  [v2] infiniband: shut up a maybe-uninitialized warning
  f2fs: replace a build-time warning with runtime WARN_ON
  ext2: avoid bogus -Wmaybe-uninitialized warning
  NFSv4.1: work around -Wmaybe-uninitialized warning
  ceph: avoid false positive maybe-uninitialized warning
  staging: lustre: restore initialization of return code
  staging: lustre: remove broken dead code in
    cfs_cpt_table_create_pattern
  UBI: fix uninitialized access of vid_hdr pointer
  block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized
  [media] rc: print correct variable for z8f0811
  [media] dib0700: fix uninitialized data on 'repeat' event
  iio: accel: sca3000_core: avoid potentially uninitialized variable
  crypto: aesni: avoid -Wmaybe-uninitialized warning
  pcmcia: fix return value of soc_pcmcia_regulator_set
  spi: fsl-espi: avoid processing uninitalized data on error
  drm: avoid uninitialized timestamp use in wait_vblank
  brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  net: bcm63xx: avoid referencing uninitialized variable
  net/hyperv: avoid uninitialized variable
  x86: apm: avoid uninitialized data
  x86: mark target address as output in 'insb' asm
  x86: math-emu: possible uninitialized variable use
  s390: pci: don't print uninitialized data for debugging
  nios2: fix timer initcall return value
  rocker: fix maybe-uninitialized warning
  Kbuild: bring back -Wmaybe-uninitialized warning

 Makefile                                           |  10 +-
 arch/arc/Makefile                                  |   4 +-
 arch/nios2/kernel/time.c                           |   1 +
 arch/s390/pci/pci_dma.c                            |   2 +-
 arch/x86/crypto/aesni-intel_glue.c                 | 121 +++++++++++++--------
 arch/x86/include/asm/io.h                          |   4 +-
 arch/x86/kernel/apm_32.c                           |   5 +-
 arch/x86/math-emu/Makefile                         |   4 +-
 arch/x86/math-emu/reg_compare.c                    |  16 +--
 drivers/block/rbd.c                                |   1 +
 drivers/gpu/drm/drm_irq.c                          |   4 +-
 drivers/infiniband/core/cma.c                      |  56 +++++-----
 drivers/media/i2c/ir-kbd-i2c.c                     |   2 +-
 drivers/media/usb/dvb-usb/dib0700_core.c           |  10 +-
 drivers/mtd/nand/mtk_ecc.c                         |  19 ++--
 drivers/mtd/ubi/eba.c                              |   2 +-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c       |   3 +-
 drivers/net/ethernet/rocker/rocker_ofdpa.c         |   4 +-
 drivers/net/hyperv/netvsc_drv.c                    |   2 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         |   2 +-
 drivers/pcmcia/soc_common.c                        |   2 +-
 drivers/spi/spi-fsl-espi.c                         |   2 +-
 drivers/staging/iio/accel/sca3000_core.c           |   2 +
 .../staging/lustre/lnet/libcfs/linux/linux-cpu.c   |   7 --
 drivers/staging/lustre/lustre/lov/lov_pack.c       |   2 +
 fs/ceph/super.c                                    |   3 +-
 fs/ext2/inode.c                                    |   7 +-
 fs/f2fs/data.c                                     |   7 ++
 fs/nfs/nfs4session.c                               |  10 +-
 net/netfilter/nft_range.c                          |  10 +-
 scripts/Makefile.ubsan                             |   4 +
 31 files changed, 187 insertions(+), 141 deletions(-)

-- 
Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 00/28] Reenable maybe-uninitialized warnings

[Arnd Bergmann]

This is a set of patches that I hope to get into v4.9 in some form
in order to turn on the -Wmaybe-uninitialized warnings again.

After talking to Linus in person at Linaro Connect about this, I
spent some time on finding all the remaining warnings, and this
is the resulting patch series. More details are in the description
of the last patch that actually enables the warning.

Let me know if there are other warnings that I missed, and whether
you think these are still appropriate for v4.9 or not.
A couple of patches are non-obvious, and could use some more
detailed review.

	Arnd

Arnd Bergmann (28):
  [v2] netfilter: nf_tables: avoid uninitialized variable warning
  [v2] mtd: mtk: avoid warning in mtk_ecc_encode
  [v2] infiniband: shut up a maybe-uninitialized warning
  f2fs: replace a build-time warning with runtime WARN_ON
  ext2: avoid bogus -Wmaybe-uninitialized warning
  NFSv4.1: work around -Wmaybe-uninitialized warning
  ceph: avoid false positive maybe-uninitialized warning
  staging: lustre: restore initialization of return code
  staging: lustre: remove broken dead code in
    cfs_cpt_table_create_pattern
  UBI: fix uninitialized access of vid_hdr pointer
  block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized
  [media] rc: print correct variable for z8f0811
  [media] dib0700: fix uninitialized data on 'repeat' event
  iio: accel: sca3000_core: avoid potentially uninitialized variable
  crypto: aesni: avoid -Wmaybe-uninitialized warning
  pcmcia: fix return value of soc_pcmcia_regulator_set
  spi: fsl-espi: avoid processing uninitalized data on error
  drm: avoid uninitialized timestamp use in wait_vblank
  brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
  net: bcm63xx: avoid referencing uninitialized variable
  net/hyperv: avoid uninitialized variable
  x86: apm: avoid uninitialized data
  x86: mark target address as output in 'insb' asm
  x86: math-emu: possible uninitialized variable use
  s390: pci: don't print uninitialized data for debugging
  nios2: fix timer initcall return value
  rocker: fix maybe-uninitialized warning
  Kbuild: bring back -Wmaybe-uninitialized warning

 Makefile                                           |  10 +-
 arch/arc/Makefile                                  |   4 +-
 arch/nios2/kernel/time.c                           |   1 +
 arch/s390/pci/pci_dma.c                            |   2 +-
 arch/x86/crypto/aesni-intel_glue.c                 | 121 +++++++++++++--------
 arch/x86/include/asm/io.h                          |   4 +-
 arch/x86/kernel/apm_32.c                           |   5 +-
 arch/x86/math-emu/Makefile                         |   4 +-
 arch/x86/math-emu/reg_compare.c                    |  16 +--
 drivers/block/rbd.c                                |   1 +
 drivers/gpu/drm/drm_irq.c                          |   4 +-
 drivers/infiniband/core/cma.c                      |  56 +++++-----
 drivers/media/i2c/ir-kbd-i2c.c                     |   2 +-
 drivers/media/usb/dvb-usb/dib0700_core.c           |  10 +-
 drivers/mtd/nand/mtk_ecc.c                         |  19 ++--
 drivers/mtd/ubi/eba.c                              |   2 +-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c       |   3 +-
 drivers/net/ethernet/rocker/rocker_ofdpa.c         |   4 +-
 drivers/net/hyperv/netvsc_drv.c                    |   2 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         |   2 +-
 drivers/pcmcia/soc_common.c                        |   2 +-
 drivers/spi/spi-fsl-espi.c                         |   2 +-
 drivers/staging/iio/accel/sca3000_core.c           |   2 +
 .../staging/lustre/lnet/libcfs/linux/linux-cpu.c   |   7 --
 drivers/staging/lustre/lustre/lov/lov_pack.c       |   2 +
 fs/ceph/super.c                                    |   3 +-
 fs/ext2/inode.c                                    |   7 +-
 fs/f2fs/data.c                                     |   7 ++
 fs/nfs/nfs4session.c                               |  10 +-
 net/netfilter/nft_range.c                          |  10 +-
 scripts/Makefile.ubsan                             |   4 +
 31 files changed, 187 insertions(+), 141 deletions(-)

-- 
Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
2.9.0

[PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning

[Arnd Bergmann]

The newly added nft_range_eval() function handles the two possible
nft range operations, but as the compiler warning points out,
any unexpected value would lead to the 'mismatch' variable being
used without being initialized:

net/netfilter/nft_range.c: In function 'nft_range_eval':
net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This removes the variable in question and instead moves the
condition into the switch itself, which is potentially more
efficient than adding a bogus 'default' clause as in my
first approach, and is nicer than using the 'uninitialized_var'
macro.

Fixes: 0f3cd9b36977 ("netfilter: nf_tables: add range expression")
Link: http://patchwork.ozlabs.org/patch/677114/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/netfilter/nft_range.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Cc: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nft_range.c b/net/netfilter/nft_range.c
index c6d5358..2dd80f4 100644
--- a/net/netfilter/nft_range.c
+++ b/net/netfilter/nft_range.c
@@ -28,22 +28,20 @@ static void nft_range_eval(const struct nft_expr *expr,
 			 const struct nft_pktinfo *pkt)
 {
 	const struct nft_range_expr *priv = nft_expr_priv(expr);
-	bool mismatch;
 	int d1, d2;
 
 	d1 = memcmp(&regs->data[priv->sreg], &priv->data_from, priv->len);
 	d2 = memcmp(&regs->data[priv->sreg], &priv->data_to, priv->len);
 	switch (priv->op) {
 	case NFT_RANGE_EQ:
-		mismatch = (d1 < 0 || d2 > 0);
+		if (d1 < 0 || d2 > 0)
+			regs->verdict.code = NFT_BREAK;
 		break;
 	case NFT_RANGE_NEQ:
-		mismatch = (d1 >= 0 && d2 <= 0);
+		if (d1 >= 0 && d2 <= 0)
+			regs->verdict.code = NFT_BREAK;
 		break;
 	}
-
-	if (mismatch)
-		regs->verdict.code = NFT_BREAK;
 }
 
 static const struct nla_policy nft_range_policy[NFTA_RANGE_MAX + 1] = {
-- 
2.9.0

Re: [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning

[Pablo Neira Ayuso]

On Tue, Oct 18, 2016 at 12:05:30AM +0200, Arnd Bergmann wrote:
> The newly added nft_range_eval() function handles the two possible
> nft range operations, but as the compiler warning points out,
> any unexpected value would lead to the 'mismatch' variable being
> used without being initialized:
> 
> net/netfilter/nft_range.c: In function 'nft_range_eval':
> net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This removes the variable in question and instead moves the
> condition into the switch itself, which is potentially more
> efficient than adding a bogus 'default' clause as in my
> first approach, and is nicer than using the 'uninitialized_var'
> macro.

Applied to the nf tree, thanks Arnd.

[PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[Arnd Bergmann]

When building with -Wmaybe-uninitialized, gcc produces a silly false positive
warning for the mtk_ecc_encode function:

drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]

The function for some reason contains a double byte swap on big-endian
builds to get the OOB data into the correct order again, and is written
in a slightly confusing way.

Using a simple memcpy32_fromio() to read the data simplifies it a lot
so it becomes more readable and produces no warning. However, the
output might not have 32-bit alignment, so we have to use another
memcpy to avoid taking alignment faults or writing beyond the end
of the array.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: move temporary buffer into struct mtk_ecc instead of having it
    on the stack, as suggested by Boris Brezillon
---
 drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index d54f666..dbf2562 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -86,6 +86,8 @@ struct mtk_ecc {
 	struct completion done;
 	struct mutex lock;
 	u32 sectors;
+
+	u8 eccdata[112];
 };
 
 static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
@@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 		   u8 *data, u32 bytes)
 {
 	dma_addr_t addr;
-	u8 *p;
-	u32 len, i, val;
-	int ret = 0;
+	u32 len;
+	int ret;
 
 	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
 	ret = dma_mapping_error(ecc->dev, addr);
@@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 
 	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
 	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
-	p = data + bytes;
 
-	/* write the parity bytes generated by the ECC back to the OOB region */
-	for (i = 0; i < len; i++) {
-		if ((i % 4) == 0)
-			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
-		p[i] = (val >> ((i % 4) * 8)) & 0xff;
-	}
+	/* write the parity bytes generated by the ECC back to temp buffer */
+	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
+
+	/* copy into possibly unaligned OOB region with actual length */
+	memcpy(data + bytes, ecc->eccdata, len);
 timeout:
 
 	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
-- 
2.9.0

[PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[Arnd Bergmann]

When building with -Wmaybe-uninitialized, gcc produces a silly false positive
warning for the mtk_ecc_encode function:

drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]

The function for some reason contains a double byte swap on big-endian
builds to get the OOB data into the correct order again, and is written
in a slightly confusing way.

Using a simple memcpy32_fromio() to read the data simplifies it a lot
so it becomes more readable and produces no warning. However, the
output might not have 32-bit alignment, so we have to use another
memcpy to avoid taking alignment faults or writing beyond the end
of the array.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: move temporary buffer into struct mtk_ecc instead of having it
    on the stack, as suggested by Boris Brezillon
---
 drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index d54f666..dbf2562 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -86,6 +86,8 @@ struct mtk_ecc {
 	struct completion done;
 	struct mutex lock;
 	u32 sectors;
+
+	u8 eccdata[112];
 };
 
 static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
@@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 		   u8 *data, u32 bytes)
 {
 	dma_addr_t addr;
-	u8 *p;
-	u32 len, i, val;
-	int ret = 0;
+	u32 len;
+	int ret;
 
 	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
 	ret = dma_mapping_error(ecc->dev, addr);
@@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 
 	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
 	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
-	p = data + bytes;
 
-	/* write the parity bytes generated by the ECC back to the OOB region */
-	for (i = 0; i < len; i++) {
-		if ((i % 4) == 0)
-			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
-		p[i] = (val >> ((i % 4) * 8)) & 0xff;
-	}
+	/* write the parity bytes generated by the ECC back to temp buffer */
+	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
+
+	/* copy into possibly unaligned OOB region with actual length */
+	memcpy(data + bytes, ecc->eccdata, len);
 timeout:
 
 	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
-- 
2.9.0

[PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[Arnd Bergmann]

When building with -Wmaybe-uninitialized, gcc produces a silly false positive
warning for the mtk_ecc_encode function:

drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]

The function for some reason contains a double byte swap on big-endian
builds to get the OOB data into the correct order again, and is written
in a slightly confusing way.

Using a simple memcpy32_fromio() to read the data simplifies it a lot
so it becomes more readable and produces no warning. However, the
output might not have 32-bit alignment, so we have to use another
memcpy to avoid taking alignment faults or writing beyond the end
of the array.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
v2: move temporary buffer into struct mtk_ecc instead of having it
    on the stack, as suggested by Boris Brezillon
---
 drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index d54f666..dbf2562 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -86,6 +86,8 @@ struct mtk_ecc {
 	struct completion done;
 	struct mutex lock;
 	u32 sectors;
+
+	u8 eccdata[112];
 };
 
 static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
@@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 		   u8 *data, u32 bytes)
 {
 	dma_addr_t addr;
-	u8 *p;
-	u32 len, i, val;
-	int ret = 0;
+	u32 len;
+	int ret;
 
 	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
 	ret = dma_mapping_error(ecc->dev, addr);
@@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 
 	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
 	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
-	p = data + bytes;
 
-	/* write the parity bytes generated by the ECC back to the OOB region */
-	for (i = 0; i < len; i++) {
-		if ((i % 4) == 0)
-			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
-		p[i] = (val >> ((i % 4) * 8)) & 0xff;
-	}
+	/* write the parity bytes generated by the ECC back to temp buffer */
+	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
+
+	/* copy into possibly unaligned OOB region with actual length */
+	memcpy(data + bytes, ecc->eccdata, len);
 timeout:
 
 	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
-- 
2.9.0

Re: [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[Boris Brezillon]

On Tue, 18 Oct 2016 00:05:31 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> warning for the mtk_ecc_encode function:
> 
> drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> The function for some reason contains a double byte swap on big-endian
> builds to get the OOB data into the correct order again, and is written
> in a slightly confusing way.
> 
> Using a simple memcpy32_fromio() to read the data simplifies it a lot
> so it becomes more readable and produces no warning. However, the
> output might not have 32-bit alignment, so we have to use another
> memcpy to avoid taking alignment faults or writing beyond the end
> of the array.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?

> ---
> v2: move temporary buffer into struct mtk_ecc instead of having it
>     on the stack, as suggested by Boris Brezillon
> ---
>  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> index d54f666..dbf2562 100644
> --- a/drivers/mtd/nand/mtk_ecc.c
> +++ b/drivers/mtd/nand/mtk_ecc.c
> @@ -86,6 +86,8 @@ struct mtk_ecc {
>  	struct completion done;
>  	struct mutex lock;
>  	u32 sectors;
> +
> +	u8 eccdata[112];
>  };
>  
>  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
>  		   u8 *data, u32 bytes)
>  {
>  	dma_addr_t addr;
> -	u8 *p;
> -	u32 len, i, val;
> -	int ret = 0;
> +	u32 len;
> +	int ret;
>  
>  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
>  	ret = dma_mapping_error(ecc->dev, addr);
> @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
>  
>  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
>  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> -	p = data + bytes;
>  
> -	/* write the parity bytes generated by the ECC back to the OOB region */
> -	for (i = 0; i < len; i++) {
> -		if ((i % 4) == 0)
> -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> -	}
> +	/* write the parity bytes generated by the ECC back to temp buffer */
> +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> +
> +	/* copy into possibly unaligned OOB region with actual length */
> +	memcpy(data + bytes, ecc->eccdata, len);
>  timeout:
>  
>  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);

[PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[Boris Brezillon]

On Tue, 18 Oct 2016 00:05:31 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> warning for the mtk_ecc_encode function:
> 
> drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> The function for some reason contains a double byte swap on big-endian
> builds to get the OOB data into the correct order again, and is written
> in a slightly confusing way.
> 
> Using a simple memcpy32_fromio() to read the data simplifies it a lot
> so it becomes more readable and produces no warning. However, the
> output might not have 32-bit alignment, so we have to use another
> memcpy to avoid taking alignment faults or writing beyond the end
> of the array.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?

> ---
> v2: move temporary buffer into struct mtk_ecc instead of having it
>     on the stack, as suggested by Boris Brezillon
> ---
>  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> index d54f666..dbf2562 100644
> --- a/drivers/mtd/nand/mtk_ecc.c
> +++ b/drivers/mtd/nand/mtk_ecc.c
> @@ -86,6 +86,8 @@ struct mtk_ecc {
>  	struct completion done;
>  	struct mutex lock;
>  	u32 sectors;
> +
> +	u8 eccdata[112];
>  };
>  
>  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
>  		   u8 *data, u32 bytes)
>  {
>  	dma_addr_t addr;
> -	u8 *p;
> -	u32 len, i, val;
> -	int ret = 0;
> +	u32 len;
> +	int ret;
>  
>  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
>  	ret = dma_mapping_error(ecc->dev, addr);
> @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
>  
>  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
>  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> -	p = data + bytes;
>  
> -	/* write the parity bytes generated by the ECC back to the OOB region */
> -	for (i = 0; i < len; i++) {
> -		if ((i % 4) == 0)
> -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> -	}
> +	/* write the parity bytes generated by the ECC back to temp buffer */
> +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> +
> +	/* copy into possibly unaligned OOB region with actual length */
> +	memcpy(data + bytes, ecc->eccdata, len);
>  timeout:
>  
>  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);

Re: [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[RogerCC.Lin]

On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> On Tue, 18 Oct 2016 00:05:31 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > warning for the mtk_ecc_encode function:
> > 
> > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 
> > The function for some reason contains a double byte swap on big-endian
> > builds to get the OOB data into the correct order again, and is written
> > in a slightly confusing way.
> > 
> > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > so it becomes more readable and produces no warning. However, the
> > output might not have 32-bit alignment, so we have to use another
> > memcpy to avoid taking alignment faults or writing beyond the end
> > of the array.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?
Tested, this patch is OK,
Tested-by: RogerCC Lin <rogercc.lin@mediatek.com>

> 
> > ---
> > v2: move temporary buffer into struct mtk_ecc instead of having it
> >     on the stack, as suggested by Boris Brezillon
> > ---
> >  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> >  1 file changed, 9 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > index d54f666..dbf2562 100644
> > --- a/drivers/mtd/nand/mtk_ecc.c
> > +++ b/drivers/mtd/nand/mtk_ecc.c
> > @@ -86,6 +86,8 @@ struct mtk_ecc {
> >  	struct completion done;
> >  	struct mutex lock;
> >  	u32 sectors;
> > +
> > +	u8 eccdata[112];
> >  };
> >  
> >  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> >  		   u8 *data, u32 bytes)
> >  {
> >  	dma_addr_t addr;
> > -	u8 *p;
> > -	u32 len, i, val;
> > -	int ret = 0;
> > +	u32 len;
> > +	int ret;
> >  
> >  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> >  	ret = dma_mapping_error(ecc->dev, addr);
> > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> >  
> >  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> >  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > -	p = data + bytes;
> >  
> > -	/* write the parity bytes generated by the ECC back to the OOB region */
> > -	for (i = 0; i < len; i++) {
> > -		if ((i % 4) == 0)
> > -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > -	}
> > +	/* write the parity bytes generated by the ECC back to temp buffer */
> > +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > +
> > +	/* copy into possibly unaligned OOB region with actual length */
> > +	memcpy(data + bytes, ecc->eccdata, len);
> >  timeout:
> >  
> >  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
> 

[PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[RogerCC.Lin]

On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> On Tue, 18 Oct 2016 00:05:31 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > warning for the mtk_ecc_encode function:
> > 
> > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 
> > The function for some reason contains a double byte swap on big-endian
> > builds to get the OOB data into the correct order again, and is written
> > in a slightly confusing way.
> > 
> > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > so it becomes more readable and produces no warning. However, the
> > output might not have 32-bit alignment, so we have to use another
> > memcpy to avoid taking alignment faults or writing beyond the end
> > of the array.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?
Tested, this patch is OK,
Tested-by: RogerCC Lin <rogercc.lin@mediatek.com>

> 
> > ---
> > v2: move temporary buffer into struct mtk_ecc instead of having it
> >     on the stack, as suggested by Boris Brezillon
> > ---
> >  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> >  1 file changed, 9 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > index d54f666..dbf2562 100644
> > --- a/drivers/mtd/nand/mtk_ecc.c
> > +++ b/drivers/mtd/nand/mtk_ecc.c
> > @@ -86,6 +86,8 @@ struct mtk_ecc {
> >  	struct completion done;
> >  	struct mutex lock;
> >  	u32 sectors;
> > +
> > +	u8 eccdata[112];
> >  };
> >  
> >  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> >  		   u8 *data, u32 bytes)
> >  {
> >  	dma_addr_t addr;
> > -	u8 *p;
> > -	u32 len, i, val;
> > -	int ret = 0;
> > +	u32 len;
> > +	int ret;
> >  
> >  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> >  	ret = dma_mapping_error(ecc->dev, addr);
> > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> >  
> >  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> >  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > -	p = data + bytes;
> >  
> > -	/* write the parity bytes generated by the ECC back to the OOB region */
> > -	for (i = 0; i < len; i++) {
> > -		if ((i % 4) == 0)
> > -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > -	}
> > +	/* write the parity bytes generated by the ECC back to temp buffer */
> > +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > +
> > +	/* copy into possibly unaligned OOB region with actual length */
> > +	memcpy(data + bytes, ecc->eccdata, len);
> >  timeout:
> >  
> >  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
> 

Re: [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[RogerCC.Lin]

On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> On Tue, 18 Oct 2016 00:05:31 +0200
> Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> 
> > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > warning for the mtk_ecc_encode function:
> > 
> > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 
> > The function for some reason contains a double byte swap on big-endian
> > builds to get the OOB data into the correct order again, and is written
> > in a slightly confusing way.
> > 
> > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > so it becomes more readable and produces no warning. However, the
> > output might not have 32-bit alignment, so we have to use another
> > memcpy to avoid taking alignment faults or writing beyond the end
> > of the array.
> > 
> > Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?
Tested, this patch is OK,
Tested-by: RogerCC Lin <rogercc.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

> 
> > ---
> > v2: move temporary buffer into struct mtk_ecc instead of having it
> >     on the stack, as suggested by Boris Brezillon
> > ---
> >  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> >  1 file changed, 9 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > index d54f666..dbf2562 100644
> > --- a/drivers/mtd/nand/mtk_ecc.c
> > +++ b/drivers/mtd/nand/mtk_ecc.c
> > @@ -86,6 +86,8 @@ struct mtk_ecc {
> >  	struct completion done;
> >  	struct mutex lock;
> >  	u32 sectors;
> > +
> > +	u8 eccdata[112];
> >  };
> >  
> >  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> >  		   u8 *data, u32 bytes)
> >  {
> >  	dma_addr_t addr;
> > -	u8 *p;
> > -	u32 len, i, val;
> > -	int ret = 0;
> > +	u32 len;
> > +	int ret;
> >  
> >  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> >  	ret = dma_mapping_error(ecc->dev, addr);
> > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> >  
> >  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> >  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > -	p = data + bytes;
> >  
> > -	/* write the parity bytes generated by the ECC back to the OOB region */
> > -	for (i = 0; i < len; i++) {
> > -		if ((i % 4) == 0)
> > -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > -	}
> > +	/* write the parity bytes generated by the ECC back to temp buffer */
> > +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > +
> > +	/* copy into possibly unaligned OOB region with actual length */
> > +	memcpy(data + bytes, ecc->eccdata, len);
> >  timeout:
> >  
> >  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
> 

Re: [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[Boris Brezillon]

On Tue, 18 Oct 2016 18:12:32 +0800
RogerCC.Lin <rogercc.lin@mediatek.com> wrote:

> On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> > On Tue, 18 Oct 2016 00:05:31 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >   
> > > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > > warning for the mtk_ecc_encode function:
> > > 
> > > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > 
> > > The function for some reason contains a double byte swap on big-endian
> > > builds to get the OOB data into the correct order again, and is written
> > > in a slightly confusing way.
> > > 
> > > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > > so it becomes more readable and produces no warning. However, the
> > > output might not have 32-bit alignment, so we have to use another
> > > memcpy to avoid taking alignment faults or writing beyond the end
> > > of the array.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>  
> > 
> > Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?  
> Tested, this patch is OK,
> Tested-by: RogerCC Lin <rogercc.lin@mediatek.com>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Brian, can you take this patch for the next -rc?

> 
> >   
> > > ---
> > > v2: move temporary buffer into struct mtk_ecc instead of having it
> > >     on the stack, as suggested by Boris Brezillon
> > > ---
> > >  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> > >  1 file changed, 9 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > > index d54f666..dbf2562 100644
> > > --- a/drivers/mtd/nand/mtk_ecc.c
> > > +++ b/drivers/mtd/nand/mtk_ecc.c
> > > @@ -86,6 +86,8 @@ struct mtk_ecc {
> > >  	struct completion done;
> > >  	struct mutex lock;
> > >  	u32 sectors;
> > > +
> > > +	u8 eccdata[112];
> > >  };
> > >  
> > >  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >  		   u8 *data, u32 bytes)
> > >  {
> > >  	dma_addr_t addr;
> > > -	u8 *p;
> > > -	u32 len, i, val;
> > > -	int ret = 0;
> > > +	u32 len;
> > > +	int ret;
> > >  
> > >  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> > >  	ret = dma_mapping_error(ecc->dev, addr);
> > > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >  
> > >  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> > >  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > > -	p = data + bytes;
> > >  
> > > -	/* write the parity bytes generated by the ECC back to the OOB region */
> > > -	for (i = 0; i < len; i++) {
> > > -		if ((i % 4) == 0)
> > > -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > > -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > > -	}
> > > +	/* write the parity bytes generated by the ECC back to temp buffer */
> > > +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > > +
> > > +	/* copy into possibly unaligned OOB region with actual length */
> > > +	memcpy(data + bytes, ecc->eccdata, len);
> > >  timeout:
> > >  
> > >  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);  
> >   
> 
> 

[PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[Boris Brezillon]

On Tue, 18 Oct 2016 18:12:32 +0800
RogerCC.Lin <rogercc.lin@mediatek.com> wrote:

> On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> > On Tue, 18 Oct 2016 00:05:31 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >   
> > > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > > warning for the mtk_ecc_encode function:
> > > 
> > > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > 
> > > The function for some reason contains a double byte swap on big-endian
> > > builds to get the OOB data into the correct order again, and is written
> > > in a slightly confusing way.
> > > 
> > > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > > so it becomes more readable and produces no warning. However, the
> > > output might not have 32-bit alignment, so we have to use another
> > > memcpy to avoid taking alignment faults or writing beyond the end
> > > of the array.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>  
> > 
> > Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?  
> Tested, this patch is OK,
> Tested-by: RogerCC Lin <rogercc.lin@mediatek.com>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Brian, can you take this patch for the next -rc?

> 
> >   
> > > ---
> > > v2: move temporary buffer into struct mtk_ecc instead of having it
> > >     on the stack, as suggested by Boris Brezillon
> > > ---
> > >  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> > >  1 file changed, 9 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > > index d54f666..dbf2562 100644
> > > --- a/drivers/mtd/nand/mtk_ecc.c
> > > +++ b/drivers/mtd/nand/mtk_ecc.c
> > > @@ -86,6 +86,8 @@ struct mtk_ecc {
> > >  	struct completion done;
> > >  	struct mutex lock;
> > >  	u32 sectors;
> > > +
> > > +	u8 eccdata[112];
> > >  };
> > >  
> > >  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >  		   u8 *data, u32 bytes)
> > >  {
> > >  	dma_addr_t addr;
> > > -	u8 *p;
> > > -	u32 len, i, val;
> > > -	int ret = 0;
> > > +	u32 len;
> > > +	int ret;
> > >  
> > >  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> > >  	ret = dma_mapping_error(ecc->dev, addr);
> > > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >  
> > >  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> > >  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > > -	p = data + bytes;
> > >  
> > > -	/* write the parity bytes generated by the ECC back to the OOB region */
> > > -	for (i = 0; i < len; i++) {
> > > -		if ((i % 4) == 0)
> > > -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > > -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > > -	}
> > > +	/* write the parity bytes generated by the ECC back to temp buffer */
> > > +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > > +
> > > +	/* copy into possibly unaligned OOB region with actual length */
> > > +	memcpy(data + bytes, ecc->eccdata, len);
> > >  timeout:
> > >  
> > >  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);  
> >   
> 
> 

Re: [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode

[Boris Brezillon]

On Tue, 18 Oct 2016 18:12:32 +0800
RogerCC.Lin <rogercc.lin@mediatek.com> wrote:

> On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> > On Tue, 18 Oct 2016 00:05:31 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >   
> > > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > > warning for the mtk_ecc_encode function:
> > > 
> > > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > 
> > > The function for some reason contains a double byte swap on big-endian
> > > builds to get the OOB data into the correct order again, and is written
> > > in a slightly confusing way.
> > > 
> > > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > > so it becomes more readable and produces no warning. However, the
> > > output might not have 32-bit alignment, so we have to use another
> > > memcpy to avoid taking alignment faults or writing beyond the end
> > > of the array.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>  
> > 
> > Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?  
> Tested, this patch is OK,
> Tested-by: RogerCC Lin <rogercc.lin@mediatek.com>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Brian, can you take this patch for the next -rc?

> 
> >   
> > > ---
> > > v2: move temporary buffer into struct mtk_ecc instead of having it
> > >     on the stack, as suggested by Boris Brezillon
> > > ---
> > >  drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> > >  1 file changed, 9 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > > index d54f666..dbf2562 100644
> > > --- a/drivers/mtd/nand/mtk_ecc.c
> > > +++ b/drivers/mtd/nand/mtk_ecc.c
> > > @@ -86,6 +86,8 @@ struct mtk_ecc {
> > >  	struct completion done;
> > >  	struct mutex lock;
> > >  	u32 sectors;
> > > +
> > > +	u8 eccdata[112];
> > >  };
> > >  
> > >  static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >  		   u8 *data, u32 bytes)
> > >  {
> > >  	dma_addr_t addr;
> > > -	u8 *p;
> > > -	u32 len, i, val;
> > > -	int ret = 0;
> > > +	u32 len;
> > > +	int ret;
> > >  
> > >  	addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> > >  	ret = dma_mapping_error(ecc->dev, addr);
> > > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >  
> > >  	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> > >  	len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > > -	p = data + bytes;
> > >  
> > > -	/* write the parity bytes generated by the ECC back to the OOB region */
> > > -	for (i = 0; i < len; i++) {
> > > -		if ((i % 4) == 0)
> > > -			val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > > -		p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > > -	}
> > > +	/* write the parity bytes generated by the ECC back to temp buffer */
> > > +	__ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > > +
> > > +	/* copy into possibly unaligned OOB region with actual length */
> > > +	memcpy(data + bytes, ecc->eccdata, len);
> > >  timeout:
> > >  
> > >  	dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);  
> >   
> 
> 

[PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning

[Arnd Bergmann]

Some configurations produce this harmless warning when built with
gcc -Wmaybe-uninitialized:

infiniband/core/cma.c: In function 'cma_get_net_dev':
infiniband/core/cma.c:1242:12: warning: 'src_addr_storage.sin_addr.s_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]

I previously reported this for the powerpc64 defconfig, but have now
reproduced the same thing for x86 as well, using gcc-5 or higher.

The code looks correct to me, and this change just rearranges it
by making sure we alway initialize the entire address structure
to make the warning disappear. My first approach added an
initialization at the time of the declaration, which Doug commented
may be too costly, so I hope this version doesn't add overhead.

Link: http://arm-soc.lixom.net/buildlogs/mainline/v4.7-rc6/buildall.powerpc.ppc64_defconfig.log.passed
Link: https://patchwork.kernel.org/patch/9212825/
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 56 ++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 36bf50e..24e0ea6 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1094,47 +1094,47 @@ static void cma_save_ib_info(struct sockaddr *src_addr,
 	}
 }
 
-static void cma_save_ip4_info(struct sockaddr *src_addr,
-			      struct sockaddr *dst_addr,
+static void cma_save_ip4_info(struct sockaddr_in *src_addr,
+			      struct sockaddr_in *dst_addr,
 			      struct cma_hdr *hdr,
 			      __be16 local_port)
 {
-	struct sockaddr_in *ip4;
-
 	if (src_addr) {
-		ip4 = (struct sockaddr_in *)src_addr;
-		ip4->sin_family = AF_INET;
-		ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
-		ip4->sin_port = local_port;
+		*src_addr = (struct sockaddr_in) {
+			.sin_family = AF_INET,
+			.sin_addr.s_addr = hdr->dst_addr.ip4.addr,
+			.sin_port = local_port,
+		};
 	}
 
 	if (dst_addr) {
-		ip4 = (struct sockaddr_in *)dst_addr;
-		ip4->sin_family = AF_INET;
-		ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
-		ip4->sin_port = hdr->port;
+		*dst_addr = (struct sockaddr_in) {
+			.sin_family = AF_INET,
+			.sin_addr.s_addr = hdr->src_addr.ip4.addr,
+			.sin_port = hdr->port,
+		};
 	}
 }
 
-static void cma_save_ip6_info(struct sockaddr *src_addr,
-			      struct sockaddr *dst_addr,
+static void cma_save_ip6_info(struct sockaddr_in6 *src_addr,
+			      struct sockaddr_in6 *dst_addr,
 			      struct cma_hdr *hdr,
 			      __be16 local_port)
 {
-	struct sockaddr_in6 *ip6;
-
 	if (src_addr) {
-		ip6 = (struct sockaddr_in6 *)src_addr;
-		ip6->sin6_family = AF_INET6;
-		ip6->sin6_addr = hdr->dst_addr.ip6;
-		ip6->sin6_port = local_port;
+		*src_addr = (struct sockaddr_in6) {
+			.sin6_family = AF_INET6,
+			.sin6_addr = hdr->dst_addr.ip6,
+			.sin6_port = local_port,
+		};
 	}
 
 	if (dst_addr) {
-		ip6 = (struct sockaddr_in6 *)dst_addr;
-		ip6->sin6_family = AF_INET6;
-		ip6->sin6_addr = hdr->src_addr.ip6;
-		ip6->sin6_port = hdr->port;
+		*dst_addr = (struct sockaddr_in6) {
+			.sin6_family = AF_INET6,
+			.sin6_addr = hdr->src_addr.ip6,
+			.sin6_port = hdr->port,
+		};
 	}
 }
 
@@ -1159,10 +1159,12 @@ static int cma_save_ip_info(struct sockaddr *src_addr,
 
 	switch (cma_get_ip_ver(hdr)) {
 	case 4:
-		cma_save_ip4_info(src_addr, dst_addr, hdr, port);
+		cma_save_ip4_info((struct sockaddr_in *)src_addr,
+				  (struct sockaddr_in *)dst_addr, hdr, port);
 		break;
 	case 6:
-		cma_save_ip6_info(src_addr, dst_addr, hdr, port);
+		cma_save_ip6_info((struct sockaddr_in6 *)src_addr,
+				  (struct sockaddr_in6 *)dst_addr, hdr, port);
 		break;
 	default:
 		return -EAFNOSUPPORT;
@@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
 static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
 					  const struct cma_req_info *req)
 {
-	struct sockaddr_storage listen_addr_storage, src_addr_storage;
+	struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
 	struct sockaddr *listen_addr = (struct sockaddr *)&listen_addr_storage,
 			*src_addr = (struct sockaddr *)&src_addr_storage;
 	struct net_device *net_dev;
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning

[Arnd Bergmann]

Some configurations produce this harmless warning when built with
gcc -Wmaybe-uninitialized:

infiniband/core/cma.c: In function 'cma_get_net_dev':
infiniband/core/cma.c:1242:12: warning: 'src_addr_storage.sin_addr.s_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]

I previously reported this for the powerpc64 defconfig, but have now
reproduced the same thing for x86 as well, using gcc-5 or higher.

The code looks correct to me, and this change just rearranges it
by making sure we alway initialize the entire address structure
to make the warning disappear. My first approach added an
initialization at the time of the declaration, which Doug commented
may be too costly, so I hope this version doesn't add overhead.

Link: http://arm-soc.lixom.net/buildlogs/mainline/v4.7-rc6/buildall.powerpc.ppc64_defconfig.log.passed
Link: https://patchwork.kernel.org/patch/9212825/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/infiniband/core/cma.c | 56 ++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 36bf50e..24e0ea6 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1094,47 +1094,47 @@ static void cma_save_ib_info(struct sockaddr *src_addr,
 	}
 }
 
-static void cma_save_ip4_info(struct sockaddr *src_addr,
-			      struct sockaddr *dst_addr,
+static void cma_save_ip4_info(struct sockaddr_in *src_addr,
+			      struct sockaddr_in *dst_addr,
 			      struct cma_hdr *hdr,
 			      __be16 local_port)
 {
-	struct sockaddr_in *ip4;
-
 	if (src_addr) {
-		ip4 = (struct sockaddr_in *)src_addr;
-		ip4->sin_family = AF_INET;
-		ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
-		ip4->sin_port = local_port;
+		*src_addr = (struct sockaddr_in) {
+			.sin_family = AF_INET,
+			.sin_addr.s_addr = hdr->dst_addr.ip4.addr,
+			.sin_port = local_port,
+		};
 	}
 
 	if (dst_addr) {
-		ip4 = (struct sockaddr_in *)dst_addr;
-		ip4->sin_family = AF_INET;
-		ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
-		ip4->sin_port = hdr->port;
+		*dst_addr = (struct sockaddr_in) {
+			.sin_family = AF_INET,
+			.sin_addr.s_addr = hdr->src_addr.ip4.addr,
+			.sin_port = hdr->port,
+		};
 	}
 }
 
-static void cma_save_ip6_info(struct sockaddr *src_addr,
-			      struct sockaddr *dst_addr,
+static void cma_save_ip6_info(struct sockaddr_in6 *src_addr,
+			      struct sockaddr_in6 *dst_addr,
 			      struct cma_hdr *hdr,
 			      __be16 local_port)
 {
-	struct sockaddr_in6 *ip6;
-
 	if (src_addr) {
-		ip6 = (struct sockaddr_in6 *)src_addr;
-		ip6->sin6_family = AF_INET6;
-		ip6->sin6_addr = hdr->dst_addr.ip6;
-		ip6->sin6_port = local_port;
+		*src_addr = (struct sockaddr_in6) {
+			.sin6_family = AF_INET6,
+			.sin6_addr = hdr->dst_addr.ip6,
+			.sin6_port = local_port,
+		};
 	}
 
 	if (dst_addr) {
-		ip6 = (struct sockaddr_in6 *)dst_addr;
-		ip6->sin6_family = AF_INET6;
-		ip6->sin6_addr = hdr->src_addr.ip6;
-		ip6->sin6_port = hdr->port;
+		*dst_addr = (struct sockaddr_in6) {
+			.sin6_family = AF_INET6,
+			.sin6_addr = hdr->src_addr.ip6,
+			.sin6_port = hdr->port,
+		};
 	}
 }
 
@@ -1159,10 +1159,12 @@ static int cma_save_ip_info(struct sockaddr *src_addr,
 
 	switch (cma_get_ip_ver(hdr)) {
 	case 4:
-		cma_save_ip4_info(src_addr, dst_addr, hdr, port);
+		cma_save_ip4_info((struct sockaddr_in *)src_addr,
+				  (struct sockaddr_in *)dst_addr, hdr, port);
 		break;
 	case 6:
-		cma_save_ip6_info(src_addr, dst_addr, hdr, port);
+		cma_save_ip6_info((struct sockaddr_in6 *)src_addr,
+				  (struct sockaddr_in6 *)dst_addr, hdr, port);
 		break;
 	default:
 		return -EAFNOSUPPORT;
@@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
 static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
 					  const struct cma_req_info *req)
 {
-	struct sockaddr_storage listen_addr_storage, src_addr_storage;
+	struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
 	struct sockaddr *listen_addr = (struct sockaddr *)&listen_addr_storage,
 			*src_addr = (struct sockaddr *)&src_addr_storage;
 	struct net_device *net_dev;
-- 
2.9.0

Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning

[Haggai Eran]

On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
> @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
>  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
>  					  const struct cma_req_info *req)
>  {
> -	struct sockaddr_storage listen_addr_storage, src_addr_storage;
> +	struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};

Doesn't this still translate to an extra initialization that Doug was
worried about?

Haggai
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning

[Haggai Eran]

On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
> @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
>  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
>  					  const struct cma_req_info *req)
>  {
> -	struct sockaddr_storage listen_addr_storage, src_addr_storage;
> +	struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};

Doesn't this still translate to an extra initialization that Doug was
worried about?

Haggai

Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning

[Arnd Bergmann]

On Tuesday, October 18, 2016 9:47:31 AM CEST Haggai Eran wrote:
> On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
> > @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
> >  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
> >                                         const struct cma_req_info *req)
> >  {
> > -     struct sockaddr_storage listen_addr_storage, src_addr_storage;
> > +     struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
> 
> Doesn't this still translate to an extra initialization that Doug was
> worried about?

Thanks for spotting this. I must have screwed up while rebasing the patch
at some point, this one change should not be there, the other changes by
themselves sufficiently address the warning.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning

[Arnd Bergmann]

On Tuesday, October 18, 2016 9:47:31 AM CEST Haggai Eran wrote:
> On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
> > @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
> >  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
> >                                         const struct cma_req_info *req)
> >  {
> > -     struct sockaddr_storage listen_addr_storage, src_addr_storage;
> > +     struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
> 
> Doesn't this still translate to an extra initialization that Doug was
> worried about?

Thanks for spotting this. I must have screwed up while rebasing the patch
at some point, this one change should not be there, the other changes by
themselves sufficiently address the warning.

	Arnd

Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning

[Haggai Eran]

On 10/18/2016 1:18 PM, Arnd Bergmann wrote:
> On Tuesday, October 18, 2016 9:47:31 AM CEST Haggai Eran wrote:
>> On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
>>> @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
>>>  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
>>>                                         const struct cma_req_info *req)
>>>  {
>>> -     struct sockaddr_storage listen_addr_storage, src_addr_storage;
>>> +     struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
>>
>> Doesn't this still translate to an extra initialization that Doug was
>> worried about?
> 
> Thanks for spotting this. I must have screwed up while rebasing the patch
> at some point, this one change should not be there, the other changes by
> themselves sufficiently address the warning.

Okay, other than this the patch looks good to me.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/28] [v2] infiniband: shut up a maybe-uninitialized warning

[Haggai Eran]

On 10/18/2016 1:18 PM, Arnd Bergmann wrote:
> On Tuesday, October 18, 2016 9:47:31 AM CEST Haggai Eran wrote:
>> On 10/18/2016 1:05 AM, Arnd Bergmann wrote:
>>> @@ -1309,7 +1311,7 @@ static bool validate_net_dev(struct net_device *net_dev,
>>>  static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
>>>                                         const struct cma_req_info *req)
>>>  {
>>> -     struct sockaddr_storage listen_addr_storage, src_addr_storage;
>>> +     struct sockaddr_storage listen_addr_storage = {}, src_addr_storage = {};
>>
>> Doesn't this still translate to an extra initialization that Doug was
>> worried about?
> 
> Thanks for spotting this. I must have screwed up while rebasing the patch
> at some point, this one change should not be there, the other changes by
> themselves sufficiently address the warning.

Okay, other than this the patch looks good to me.

[PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON

[Arnd Bergmann]

gcc is unsure about the use of last_ofs_in_node, which might happen
without a prior initialization:

fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {

I'm not sure about it either, so to shut up the warning I initialize
it to a known invalid -1u and later check for this, so we get a
runtime warning if we ever hit the uninitialized case.

It would be much better to reorganize the code in some form that
made it obvious to both the compiler and the reader that this
variable use it ok.

Since I only see the warning with gcc-4.9 but not any later version,
it's possible that the compiler is actually smarter than I am here
and has learned to see the code as correct, in which case this
patch could just be disregarded. It would certainly be helpful
to get an opinion from the maintainers on the matter.

Fixes: 46008c6d4232 ("f2fs: support in batch multi blocks preallocation")
Cc: Chao Yu <yuchao0@huawei.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/f2fs/data.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9ae194f..1b17de2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -696,6 +696,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		goto out;
 	}
 
+	/*
+	 * FIXME: without this, we get "warning: ‘last_ofs_in_node’ may be
+	 * used uninitialized". It's not clear whether that can actually
+	 * happen, so there is now a WARN_ON() checking for this.
+	 */
+	last_ofs_in_node = -1u;
 next_dnode:
 	if (create)
 		f2fs_lock_op(sbi);
@@ -796,6 +802,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		allocated = dn.node_changed;
 
 		map->m_len += dn.ofs_in_node - ofs_in_node;
+		WARN_ON(last_ofs_in_node == -1u);
 		if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
 			err = -ENOSPC;
 			goto sync_out;
-- 
2.9.0

[PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON

[Arnd Bergmann]

gcc is unsure about the use of last_ofs_in_node, which might happen
without a prior initialization:

fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {

I'm not sure about it either, so to shut up the warning I initialize
it to a known invalid -1u and later check for this, so we get a
runtime warning if we ever hit the uninitialized case.

It would be much better to reorganize the code in some form that
made it obvious to both the compiler and the reader that this
variable use it ok.

Since I only see the warning with gcc-4.9 but not any later version,
it's possible that the compiler is actually smarter than I am here
and has learned to see the code as correct, in which case this
patch could just be disregarded. It would certainly be helpful
to get an opinion from the maintainers on the matter.

Fixes: 46008c6d4232 ("f2fs: support in batch multi blocks preallocation")
Cc: Chao Yu <yuchao0@huawei.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/f2fs/data.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9ae194f..1b17de2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -696,6 +696,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		goto out;
 	}
 
+	/*
+	 * FIXME: without this, we get "warning: ‘last_ofs_in_node’ may be
+	 * used uninitialized". It's not clear whether that can actually
+	 * happen, so there is now a WARN_ON() checking for this.
+	 */
+	last_ofs_in_node = -1u;
 next_dnode:
 	if (create)
 		f2fs_lock_op(sbi);
@@ -796,6 +802,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		allocated = dn.node_changed;
 
 		map->m_len += dn.ofs_in_node - ofs_in_node;
+		WARN_ON(last_ofs_in_node == -1u);
 		if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
 			err = -ENOSPC;
 			goto sync_out;
-- 
2.9.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON

[Chao Yu]

Hi Arnd,

On 2016/10/18 6:05, Arnd Bergmann wrote:
> gcc is unsure about the use of last_ofs_in_node, which might happen
> without a prior initialization:
> 
> fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
> fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>    if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {

In each round of dnode block traverse, we will init 'prealloc' and then update
'prealloc' and 'last_ofs_in_node' together in below lines of f2fs_map_blocks:
			if (flag == F2FS_GET_BLOCK_PRE_AIO) {
				if (blkaddr == NULL_ADDR) {
					prealloc++;
					last_ofs_in_node = dn.ofs_in_node;
				}
			}

Then in below codes, it is safe to use 'last_ofs_in_node' since we will check
'prealloc' firstly, so if 'prealloc' is non-zero, 'last_ofs_in_node' must be valid.
	if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {

So I think we should not add WARN_ON there.

Thanks,

> 
> I'm not sure about it either, so to shut up the warning I initialize
> it to a known invalid -1u and later check for this, so we get a
> runtime warning if we ever hit the uninitialized case.
> 
> It would be much better to reorganize the code in some form that
> made it obvious to both the compiler and the reader that this
> variable use it ok.
> 
> Since I only see the warning with gcc-4.9 but not any later version,
> it's possible that the compiler is actually smarter than I am here
> and has learned to see the code as correct, in which case this
> patch could just be disregarded. It would certainly be helpful
> to get an opinion from the maintainers on the matter.
> 
> Fixes: 46008c6d4232 ("f2fs: support in batch multi blocks preallocation")
> Cc: Chao Yu <yuchao0@huawei.com>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/f2fs/data.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 9ae194f..1b17de2 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -696,6 +696,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  		goto out;
>  	}
>  
> +	/*
> +	 * FIXME: without this, we get "warning: ‘last_ofs_in_node’ may be
> +	 * used uninitialized". It's not clear whether that can actually
> +	 * happen, so there is now a WARN_ON() checking for this.
> +	 */
> +	last_ofs_in_node = -1u;
>  next_dnode:
>  	if (create)
>  		f2fs_lock_op(sbi);
> @@ -796,6 +802,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  		allocated = dn.node_changed;
>  
>  		map->m_len += dn.ofs_in_node - ofs_in_node;
> +		WARN_ON(last_ofs_in_node == -1u);
>  		if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
>  			err = -ENOSPC;
>  			goto sync_out;
> 

Re: [f2fs-dev] [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON

[Arnd Bergmann]

On Wednesday, October 26, 2016 10:05:00 PM CEST Chao Yu wrote:
> On 2016/10/18 6:05, Arnd Bergmann wrote:
> > gcc is unsure about the use of last_ofs_in_node, which might happen
> > without a prior initialization:
> > 
> > fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
> > fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> >    if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
> 
> In each round of dnode block traverse, we will init 'prealloc' and then update
> 'prealloc' and 'last_ofs_in_node' together in below lines of f2fs_map_blocks:
>                         if (flag == F2FS_GET_BLOCK_PRE_AIO) {
>                                 if (blkaddr == NULL_ADDR) {
>                                         prealloc++;
>                                         last_ofs_in_node = dn.ofs_in_node;
>                                 }
>                         }
> 
> Then in below codes, it is safe to use 'last_ofs_in_node' since we will check
> 'prealloc' firstly, so if 'prealloc' is non-zero, 'last_ofs_in_node' must be valid.
>         if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
> 
> So I think we should not add WARN_ON there.

Ok, that make sense. Thanks for taking a closer look!

Should we just set last_ofs_in_node to the same value as ofs_in_node
before the loop?

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9ae194f..14db4b7 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -716,7 +716,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 	}
 
 	prealloc = 0;
-	ofs_in_node = dn.ofs_in_node;
+	last_ofs_in_node = ofs_in_node = dn.ofs_in_node;
 	end_offset = ADDRS_PER_PAGE(dn.node_page, inode);
 
 next_block:

	Arnd

Re: [f2fs-dev] [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON

[Chao Yu]

On 2016/10/26 22:57, Arnd Bergmann wrote:
> On Wednesday, October 26, 2016 10:05:00 PM CEST Chao Yu wrote:
>> On 2016/10/18 6:05, Arnd Bergmann wrote:
>>> gcc is unsure about the use of last_ofs_in_node, which might happen
>>> without a prior initialization:
>>>
>>> fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
>>> fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>    if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
>>
>> In each round of dnode block traverse, we will init 'prealloc' and then update
>> 'prealloc' and 'last_ofs_in_node' together in below lines of f2fs_map_blocks:
>>                         if (flag == F2FS_GET_BLOCK_PRE_AIO) {
>>                                 if (blkaddr == NULL_ADDR) {
>>                                         prealloc++;
>>                                         last_ofs_in_node = dn.ofs_in_node;
>>                                 }
>>                         }
>>
>> Then in below codes, it is safe to use 'last_ofs_in_node' since we will check
>> 'prealloc' firstly, so if 'prealloc' is non-zero, 'last_ofs_in_node' must be valid.
>>         if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
>>
>> So I think we should not add WARN_ON there.
> 
> Ok, that make sense. Thanks for taking a closer look!
> 
> Should we just set last_ofs_in_node to the same value as ofs_in_node
> before the loop?

I think it's OK as it can remove warning compiler reports. :)

Thanks,

> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 9ae194f..14db4b7 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -716,7 +716,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  	}
>  
>  	prealloc = 0;
> -	ofs_in_node = dn.ofs_in_node;
> +	last_ofs_in_node = ofs_in_node = dn.ofs_in_node;
>  	end_offset = ADDRS_PER_PAGE(dn.node_page, inode);
>  
>  next_block:
> 
> 	Arnd
> 
> .
> 

Re: [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON

[Chao Yu]

On 2016/10/26 22:57, Arnd Bergmann wrote:
> On Wednesday, October 26, 2016 10:05:00 PM CEST Chao Yu wrote:
>> On 2016/10/18 6:05, Arnd Bergmann wrote:
>>> gcc is unsure about the use of last_ofs_in_node, which might happen
>>> without a prior initialization:
>>>
>>> fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
>>> fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>    if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
>>
>> In each round of dnode block traverse, we will init 'prealloc' and then update
>> 'prealloc' and 'last_ofs_in_node' together in below lines of f2fs_map_blocks:
>>                         if (flag == F2FS_GET_BLOCK_PRE_AIO) {
>>                                 if (blkaddr == NULL_ADDR) {
>>                                         prealloc++;
>>                                         last_ofs_in_node = dn.ofs_in_node;
>>                                 }
>>                         }
>>
>> Then in below codes, it is safe to use 'last_ofs_in_node' since we will check
>> 'prealloc' firstly, so if 'prealloc' is non-zero, 'last_ofs_in_node' must be valid.
>>         if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
>>
>> So I think we should not add WARN_ON there.
> 
> Ok, that make sense. Thanks for taking a closer look!
> 
> Should we just set last_ofs_in_node to the same value as ofs_in_node
> before the loop?

I think it's OK as it can remove warning compiler reports. :)

Thanks,

> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 9ae194f..14db4b7 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -716,7 +716,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>  	}
>  
>  	prealloc = 0;
> -	ofs_in_node = dn.ofs_in_node;
> +	last_ofs_in_node = ofs_in_node = dn.ofs_in_node;
>  	end_offset = ADDRS_PER_PAGE(dn.node_page, inode);
>  
>  next_block:
> 
> 	Arnd
> 
> .
> 


------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive. 
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning

[Arnd Bergmann]

On ARM, we get this false-positive warning since the rework of
the ext2_get_blocks interface:

fs/ext2/inode.c: In function 'ext2_get_block':
include/linux/buffer_head.h:340:16: error: 'bno' may be used uninitialized in this function [-Werror=maybe-uninitialized]

The calling conventions for this function are rather complex, and it's
not surprising that the compiler gets this wrong, I spent a long time
trying to understand how it all fits together myself.

This change to avoid the warning makes sure the compiler sees that we
always set 'bno' pointer whenever we have a positive return code.
The transformation is correct because we always arrive at the 'got_it'
label with a positive count that gets used as the return value, while
any branch to the 'cleanup' label has a negative or zero 'err'.

Fixes: 6750ad71986d ("ext2: stop passing buffer_head to ext2_get_blocks")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Dave Chinner <david@fromorbit.com>
---
 fs/ext2/inode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index d831e24..41b8b44 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -622,7 +622,7 @@ static int ext2_get_blocks(struct inode *inode,
 			   u32 *bno, bool *new, bool *boundary,
 			   int create)
 {
-	int err = -EIO;
+	int err;
 	int offsets[4];
 	Indirect chain[4];
 	Indirect *partial;
@@ -639,7 +639,7 @@ static int ext2_get_blocks(struct inode *inode,
 	depth = ext2_block_to_path(inode,iblock,offsets,&blocks_to_boundary);
 
 	if (depth == 0)
-		return (err);
+		return -EIO;
 
 	partial = ext2_get_branch(inode, depth, offsets, chain, &err);
 	/* Simplest case - block found, no allocation needed */
@@ -761,7 +761,6 @@ static int ext2_get_blocks(struct inode *inode,
 	ext2_splice_branch(inode, iblock, partial, indirect_blks, count);
 	mutex_unlock(&ei->truncate_mutex);
 got_it:
-	*bno = le32_to_cpu(chain[depth-1].key);
 	if (count > blocks_to_boundary)
 		*boundary = true;
 	err = count;
@@ -772,6 +771,8 @@ static int ext2_get_blocks(struct inode *inode,
 		brelse(partial->bh);
 		partial--;
 	}
+	if (err > 0)
+		*bno = le32_to_cpu(chain[depth-1].key);
 	return err;
 }
 
-- 
2.9.0

Re: [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning

[Christoph Hellwig]

Thanks Arnd, this looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 05/28] ext2: avoid bogus -Wmaybe-uninitialized warning

[Jan Kara]

On Mon 17-10-16 22:15:29, Christoph Hellwig wrote:
> Thanks Arnd, this looks fine to me:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks! I have pulled the patch into my tree and will push it to Linus.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 06/28] NFSv4.1: work around -Wmaybe-uninitialized warning

[Arnd Bergmann]

A bugfix introduced a harmless gcc warning in nfs4_slot_seqid_in_use
if we enable -Wmaybe-uninitialized again:

fs/nfs/nfs4session.c:203:54: error: 'cur_seq' may be used uninitialized in this function [-Werror=maybe-uninitialized]

gcc is not smart enough to conclude that the IS_ERR/PTR_ERR pair
results in a nonzero return value here. Using PTR_ERR_OR_ZERO()
instead makes this clear to the compiler.

The warning originally did not appear in v4.8 as it was globally
disabled, but the bugfix that introduced the warning got backported
to stable kernels which again enable it, and this is now the only
warning in the v4.7 builds.

Fixes: e09c978aae5b ("NFSv4.1: Fix Oopsable condition in server callback races")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
---
First submitted on Aug 31, but ended up not getting applied then
as the warning was disabled in v4.8-rc

 fs/nfs/nfs4session.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4session.c b/fs/nfs/nfs4session.c
index b629730..150c5a1 100644
--- a/fs/nfs/nfs4session.c
+++ b/fs/nfs/nfs4session.c
@@ -178,12 +178,14 @@ static int nfs4_slot_get_seqid(struct nfs4_slot_table  *tbl, u32 slotid,
 	__must_hold(&tbl->slot_tbl_lock)
 {
 	struct nfs4_slot *slot;
+	int ret;
 
 	slot = nfs4_lookup_slot(tbl, slotid);
-	if (IS_ERR(slot))
-		return PTR_ERR(slot);
-	*seq_nr = slot->seq_nr;
-	return 0;
+	ret = PTR_ERR_OR_ZERO(slot);
+	if (!ret)
+		*seq_nr = slot->seq_nr;
+
+	return ret;
 }
 
 /*
-- 
2.9.0

[PATCH 07/28] ceph: avoid false positive maybe-uninitialized warning

[Arnd Bergmann]

A recent rework removed the initialization of the local 'root'
variable that is returned from ceph_real_mount:

fs/ceph/super.c: In function 'ceph_mount':
fs/ceph/super.c:1016:38: error: 'root' may be used uninitialized in this function [-Werror=maybe-uninitialized]

It's not obvious to me what the correct fix is, so this just
returns the saved root as we did before.

Fixes: ce2728aaa82b ("ceph: avoid accessing / when mounting a subpath")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: "Yan, Zheng" <zyan@redhat.com>
---
 fs/ceph/super.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index a29ffce..79a4be8 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -821,7 +821,8 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc)
 	dout("mount start %p\n", fsc);
 	mutex_lock(&fsc->client->mount_mutex);
 
-	if (!fsc->sb->s_root) {
+	root = dget(fsc->sb->s_root);
+	if (!root) {
 		const char *path;
 		err = __ceph_open_session(fsc->client, started);
 		if (err < 0)
-- 
2.9.0

Re: [PATCH 07/28] ceph: avoid false positive maybe-uninitialized warning

[Yan, Zheng]

> On 18 Oct 2016, at 06:08, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> A recent rework removed the initialization of the local 'root'
> variable that is returned from ceph_real_mount:
> 
> fs/ceph/super.c: In function 'ceph_mount':
> fs/ceph/super.c:1016:38: error: 'root' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> It's not obvious to me what the correct fix is, so this just
> returns the saved root as we did before.
> 
> Fixes: ce2728aaa82b ("ceph: avoid accessing / when mounting a subpath")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: "Yan, Zheng" <zyan@redhat.com>
> ---
> fs/ceph/super.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index a29ffce..79a4be8 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -821,7 +821,8 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc)
> 	dout("mount start %p\n", fsc);
> 	mutex_lock(&fsc->client->mount_mutex);
> 
> -	if (!fsc->sb->s_root) {
> +	root = dget(fsc->sb->s_root);
> +	if (!root) {
> 		const char *path;
> 		err = __ceph_open_session(fsc->client, started);
> 		if (err < 0)
> — 

This bug has already been fixed.

Regards
Yan, Zheng

> 2.9.0
> 

[PATCH 08/28] staging: lustre: restore initialization of return code

[Arnd Bergmann]

A recent rework removed the initialization of the successful return
code from lpfc_write_firmware:

drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
drivers/scsi/lpfc/lpfc_init.c:10333:214: error: format '%ld' expects argument of type 'long int', but argument 9 has type 'size_t {aka const unsigned int}' [-Werror=format=]

This adds it back.

Fixes: e10a431b3fd0 ("staging: lustre: lov: move LSM to LOV layer")
Cc: John L. Hammond <john.hammond@intel.com>
Cc: Jinshan Xiong <jinshan.xiong@intel.com>
Cc: James Simmons <jsimmons@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/lustre/lustre/lov/lov_pack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
index be6e985..0439f54 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -474,6 +474,8 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm,
 	((struct lov_user_md *)lmmk)->lmm_stripe_count = lum.lmm_stripe_count;
 	if (copy_to_user(lump, lmmk, lmm_size))
 		rc = -EFAULT;
+	else
+		rc = 0;
 
 out_free:
 	kfree(lmmk);
-- 
2.9.0

[lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

[Arnd Bergmann]

A recent rework removed the initialization of the successful return
code from lpfc_write_firmware:

drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
drivers/scsi/lpfc/lpfc_init.c:10333:214: error: format '%ld' expects argument of type 'long int', but argument 9 has type 'size_t {aka const unsigned int}' [-Werror=format=]

This adds it back.

Fixes: e10a431b3fd0 ("staging: lustre: lov: move LSM to LOV layer")
Cc: John L. Hammond <john.hammond@intel.com>
Cc: Jinshan Xiong <jinshan.xiong@intel.com>
Cc: James Simmons <jsimmons@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/lustre/lustre/lov/lov_pack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
index be6e985..0439f54 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -474,6 +474,8 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm,
 	((struct lov_user_md *)lmmk)->lmm_stripe_count = lum.lmm_stripe_count;
 	if (copy_to_user(lump, lmmk, lmm_size))
 		rc = -EFAULT;
+	else
+		rc = 0;
 
 out_free:
 	kfree(lmmk);
-- 
2.9.0

[lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

[Patrick Farrell]

Arnd,


The description and the actual patch don't seem to match up.  Am I missing something?

________________________________
From: lustre-devel <lustre-devel-bounces@lists.lustre.org> on behalf of Arnd Bergmann <arnd@arndb.de>
Sent: Monday, October 17, 2016 5:08:55 PM
To: Oleg Drokin
Cc: devel at driverdev.osuosl.org; Arnd Bergmann; Greg Kroah-Hartman; linux-kernel at vger.kernel.org; Linus Torvalds; lustre-devel at lists.lustre.org
Subject: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

A recent rework removed the initialization of the successful return
code from lpfc_write_firmware:

drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
drivers/scsi/lpfc/lpfc_init.c:10333:214: error: format '%ld' expects argument of type 'long int', but argument 9 has type 'size_t {aka const unsigned int}' [-Werror=format=]

This adds it back.

Fixes: e10a431b3fd0 ("staging: lustre: lov: move LSM to LOV layer")
Cc: John L. Hammond <john.hammond@intel.com>
Cc: Jinshan Xiong <jinshan.xiong@intel.com>
Cc: James Simmons <jsimmons@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/lustre/lustre/lov/lov_pack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
index be6e985..0439f54 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -474,6 +474,8 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm,
         ((struct lov_user_md *)lmmk)->lmm_stripe_count = lum.lmm_stripe_count;
         if (copy_to_user(lump, lmmk, lmm_size))
                 rc = -EFAULT;
+       else
+               rc = 0;

 out_free:
         kfree(lmmk);
--
2.9.0

_______________________________________________
lustre-devel mailing list
lustre-devel at lists.lustre.org
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20161017/de8edd6e/attachment.htm>

Re: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

[Arnd Bergmann]

On Monday, October 17, 2016 10:23:00 PM CEST Patrick Farrell wrote:
> Arnd,
> 
> 
> The description and the actual patch don't seem to match up.  Am I missing something?

Sorry, I pasted the wrong error message when writing the changelog.

> From: lustre-devel <lustre-devel-bounces@lists.lustre.org> on behalf of Arnd Bergmann <arnd@arndb.de>
> Sent: Monday, October 17, 2016 5:08:55 PM
> To: Oleg Drokin
> Cc: devel@driverdev.osuosl.org; Arnd Bergmann; Greg Kroah-Hartman; linux-kernel@vger.kernel.org; Linus Torvalds; lustre-devel@lists.lustre.org
> Subject: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code
> 
> A recent rework removed the initialization of the successful return
> code from lpfc_write_firmware:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
> drivers/scsi/lpfc/lpfc_init.c:10333:214: error: format '%ld' expects argument of type 'long int', but argument 9 has type 'size_t {aka const unsigned int}' [-Werror=format=]
> 
> This adds it back.


It should have been this warning:


drivers/staging/lustre/lustre/lov/lov_pack.c: In function 'lov_getstripe':
drivers/staging/lustre/lustre/lov/lov_pack.c:426:9: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  return rc;
         ^~
drivers/staging/lustre/lustre/lov/lov_pack.c:313:6: note: 'rc' was declared here


	Arnd

[lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

[Arnd Bergmann]

On Monday, October 17, 2016 10:23:00 PM CEST Patrick Farrell wrote:
> Arnd,
> 
> 
> The description and the actual patch don't seem to match up.  Am I missing something?

Sorry, I pasted the wrong error message when writing the changelog.

> From: lustre-devel <lustre-devel-bounces@lists.lustre.org> on behalf of Arnd Bergmann <arnd@arndb.de>
> Sent: Monday, October 17, 2016 5:08:55 PM
> To: Oleg Drokin
> Cc: devel at driverdev.osuosl.org; Arnd Bergmann; Greg Kroah-Hartman; linux-kernel at vger.kernel.org; Linus Torvalds; lustre-devel at lists.lustre.org
> Subject: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code
> 
> A recent rework removed the initialization of the successful return
> code from lpfc_write_firmware:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
> drivers/scsi/lpfc/lpfc_init.c:10333:214: error: format '%ld' expects argument of type 'long int', but argument 9 has type 'size_t {aka const unsigned int}' [-Werror=format=]
> 
> This adds it back.


It should have been this warning:


drivers/staging/lustre/lustre/lov/lov_pack.c: In function 'lov_getstripe':
drivers/staging/lustre/lustre/lov/lov_pack.c:426:9: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  return rc;
         ^~
drivers/staging/lustre/lustre/lov/lov_pack.c:313:6: note: 'rc' was declared here


	Arnd

Re: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

[Linus Torvalds]

On Mon, Oct 17, 2016 at 3:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> Sorry, I pasted the wrong error message when writing the changelog.

Not just the warning, the summary above it talks about the wrong
function too. And the commit it references doesn't actually exist
either. So apparently this is against something else than my tree.

               Linus

[lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

[Linus Torvalds]

On Mon, Oct 17, 2016 at 3:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> Sorry, I pasted the wrong error message when writing the changelog.

Not just the warning, the summary above it talks about the wrong
function too. And the commit it references doesn't actually exist
either. So apparently this is against something else than my tree.

               Linus

Re: [lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

[Arnd Bergmann]

On Monday, October 17, 2016 3:37:11 PM CEST Linus Torvalds wrote:
> On Mon, Oct 17, 2016 at 3:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > Sorry, I pasted the wrong error message when writing the changelog.
> 
> Not just the warning, the summary above it talks about the wrong
> function too. And the commit it references doesn't actually exist
> either. So apparently this is against something else than my tree.

Right, it slipped in here together with the other lustre patch when
I rebased my longer series (based on linux-next) onto v4.9-rc1.

Both applied cleanly to v4.9-rc1 and they are required on linux-next
(not the version with the wrong changelog of course) but have no
effect in mainline so far.

I'll double-check the rest of the series tomorrow, to see if some
of the other patches also have the same problem and are only
needed on linux-next. I meant to send those as part of the
separate series for v4.10.

For now, it would be good to know if you see any remaining warnings
on your machine after applying the current series (with or without
the lustre patches, doesn't matter) to a test branch. Some other
patches in the series likely need to go through a second revision
anyway.

	Arnd

[lustre-devel] [PATCH 08/28] staging: lustre: restore initialization of return code

[Arnd Bergmann]

On Monday, October 17, 2016 3:37:11 PM CEST Linus Torvalds wrote:
> On Mon, Oct 17, 2016 at 3:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > Sorry, I pasted the wrong error message when writing the changelog.
> 
> Not just the warning, the summary above it talks about the wrong
> function too. And the commit it references doesn't actually exist
> either. So apparently this is against something else than my tree.

Right, it slipped in here together with the other lustre patch when
I rebased my longer series (based on linux-next) onto v4.9-rc1.

Both applied cleanly to v4.9-rc1 and they are required on linux-next
(not the version with the wrong changelog of course) but have no
effect in mainline so far.

I'll double-check the rest of the series tomorrow, to see if some
of the other patches also have the same problem and are only
needed on linux-next. I meant to send those as part of the
separate series for v4.10.

For now, it would be good to know if you see any remaining warnings
on your machine after applying the current series (with or without
the lustre patches, doesn't matter) to a test branch. Some other
patches in the series likely need to go through a second revision
anyway.

	Arnd

[PATCH 08/28 v2] staging: lustre: restore initialization of return code

[Arnd Bergmann]

A recent rework dropped the initialization of the initialization of the
successful return code in lov_getstripe:

drivers/staging/lustre/lustre/lov/lov_pack.c: In function 'lov_getstripe':
drivers/staging/lustre/lustre/lov/lov_pack.c:426:9: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/staging/lustre/lustre/lov/lov_pack.c:313:6: note: 'rc' was declared here

This adds it back.

Fixes: e10a431b3fd0 ("staging: lustre: lov: move LSM to LOV layer")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: fix embarrassing incorrect changelog

diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
index 17bceadd66f8..ccc1fae35791 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -418,6 +418,8 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm,
 	((struct lov_user_md *)lmmk)->lmm_stripe_count = lum.lmm_stripe_count;
 	if (copy_to_user(lump, lmmk, lmm_size))
 		rc = -EFAULT;
+	else
+		rc = 0;
 
 out_free:
 	kvfree(lmmk);

[lustre-devel] [PATCH 08/28 v2] staging: lustre: restore initialization of return code

[Arnd Bergmann]

A recent rework dropped the initialization of the initialization of the
successful return code in lov_getstripe:

drivers/staging/lustre/lustre/lov/lov_pack.c: In function 'lov_getstripe':
drivers/staging/lustre/lustre/lov/lov_pack.c:426:9: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/staging/lustre/lustre/lov/lov_pack.c:313:6: note: 'rc' was declared here

This adds it back.

Fixes: e10a431b3fd0 ("staging: lustre: lov: move LSM to LOV layer")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: fix embarrassing incorrect changelog

diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
index 17bceadd66f8..ccc1fae35791 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -418,6 +418,8 @@ int lov_getstripe(struct lov_object *obj, struct lov_stripe_md *lsm,
 	((struct lov_user_md *)lmmk)->lmm_stripe_count = lum.lmm_stripe_count;
 	if (copy_to_user(lump, lmmk, lmm_size))
 		rc = -EFAULT;
+	else
+		rc = 0;
 
 out_free:
 	kvfree(lmmk);

[PATCH 09/28] staging: lustre: remove broken dead code in cfs_cpt_table_create_pattern

[Arnd Bergmann]

After a recent bugfix, we get a warning about the use of an uninitialized
variable:

drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c: In function 'cfs_cpt_table_create_pattern':
drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c:833:7: error: 'str' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This part of the function used to not do anything as we would reassign
the 'str' pointer to something else right away, but now we pass an
uninitialized pointer into 'strchr', which can cause a kernel page fault
or worse.

Fixes: 239fd5d41f9b ("staging: lustre: libcfs: shortcut to create CPT from NUMA topology")
Cc: Liang Zhen <liang.zhen@intel.com>
Cc: James Simmons <jsimmons@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
index e8b1a61..1226cba 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
@@ -824,13 +824,6 @@ cfs_cpt_table_create_pattern(char *pattern)
 	int			ncpt;
 	int			c;
 
-	for (ncpt = 0;; ncpt++) { /* quick scan bracket */
-		str = strchr(str, '[');
-		if (!str)
-			break;
-		str++;
-	}
-
 	str = cfs_trimwhite(pattern);
 	if (*str == 'n' || *str == 'N') {
 		pattern = str + 1;
-- 
2.9.0

[lustre-devel] [PATCH 09/28] staging: lustre: remove broken dead code in cfs_cpt_table_create_pattern

[Arnd Bergmann]

After a recent bugfix, we get a warning about the use of an uninitialized
variable:

drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c: In function 'cfs_cpt_table_create_pattern':
drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c:833:7: error: 'str' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This part of the function used to not do anything as we would reassign
the 'str' pointer to something else right away, but now we pass an
uninitialized pointer into 'strchr', which can cause a kernel page fault
or worse.

Fixes: 239fd5d41f9b ("staging: lustre: libcfs: shortcut to create CPT from NUMA topology")
Cc: Liang Zhen <liang.zhen@intel.com>
Cc: James Simmons <jsimmons@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
index e8b1a61..1226cba 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
@@ -824,13 +824,6 @@ cfs_cpt_table_create_pattern(char *pattern)
 	int			ncpt;
 	int			c;
 
-	for (ncpt = 0;; ncpt++) { /* quick scan bracket */
-		str = strchr(str, '[');
-		if (!str)
-			break;
-		str++;
-	}
-
 	str = cfs_trimwhite(pattern);
 	if (*str == 'n' || *str == 'N') {
 		pattern = str + 1;
-- 
2.9.0

[PATCH 10/28] UBI: fix uninitialized access of vid_hdr pointer

[Arnd Bergmann]

A rework of UBI that just appeared in linux-next during the merge
window introduced caused the recover_peb to use a variable that
is never initialized as seen from this gcc warning:

drivers/mtd/ubi/eba.c: In function ‘recover_peb’:
drivers/mtd/ubi/eba.c:744:40: error: ‘vid_hdr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

It seems clear that the change to the function arguments was missing
the initialization that I'm now adding back to restore the
way the function was working before.

Fixes: 3291b52f9ff0 ("UBI: introduce the VID buffer concept")
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Richard Weinberger <richard@nod.at>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mtd/ubi/eba.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 95c4048..2e152be 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -719,7 +719,7 @@ static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum,
 			   struct ubi_vid_io_buf *vidb, bool *retry)
 {
 	struct ubi_device *ubi = vol->ubi;
-	struct ubi_vid_hdr *vid_hdr;
+	struct ubi_vid_hdr *vid_hdr = ubi_get_vid_hdr(vidb);
 	int new_pnum, err, vol_id = vol->vol_id, data_size;
 	uint32_t crc;
 
-- 
2.9.0

Re: [PATCH 10/28] UBI: fix uninitialized access of vid_hdr pointer

[Boris Brezillon]

Hi Arnd,

On Tue, 18 Oct 2016 00:10:13 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> A rework of UBI that just appeared in linux-next during the merge
> window introduced caused the recover_peb to use a variable that
> is never initialized as seen from this gcc warning:
> 
> drivers/mtd/ubi/eba.c: In function ‘recover_peb’:
> drivers/mtd/ubi/eba.c:744:40: error: ‘vid_hdr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> It seems clear that the change to the function arguments was missing
> the initialization that I'm now adding back to restore the
> way the function was working before.

Thanks for the fix, but Geert already sent a patch for this bug a few
days ago.

Regards,

Boris

> 
> Fixes: 3291b52f9ff0 ("UBI: introduce the VID buffer concept")
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Richard Weinberger <richard@nod.at>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/mtd/ubi/eba.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
> index 95c4048..2e152be 100644
> --- a/drivers/mtd/ubi/eba.c
> +++ b/drivers/mtd/ubi/eba.c
> @@ -719,7 +719,7 @@ static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum,
>  			   struct ubi_vid_io_buf *vidb, bool *retry)
>  {
>  	struct ubi_device *ubi = vol->ubi;
> -	struct ubi_vid_hdr *vid_hdr;
> +	struct ubi_vid_hdr *vid_hdr = ubi_get_vid_hdr(vidb);
>  	int new_pnum, err, vol_id = vol->vol_id, data_size;
>  	uint32_t crc;
>  

[PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized

[Arnd Bergmann]

When building with gcc-4.9 -Wmaybe-uninitialized, we get a bogus
warning in rbd_watch_cb, as the variable is not used at all
in the one case in which it is not initialized first:

drivers/block/rbd.c: In function ‘rbd_watch_cb’:
drivers/block/rbd.c:3690:5: error: ‘struct_v’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/block/rbd.c:3759:5: note: ‘struct_v’ was declared here

Later compiler versions fix this, but adding another initialization
here is harmless and lets us build cleanly with 4.9 as well.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/block/rbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index abb7162..4ab990b 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3776,6 +3776,7 @@ static void rbd_watch_cb(void *arg, u64 notify_id, u64 cookie,
 	} else {
 		/* legacy notification for header updates */
 		notify_op = RBD_NOTIFY_OP_HEADER_UPDATE;
+		struct_v = 0;
 		len = 0;
 	}
 
-- 
2.9.0

Re: [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized

[Ilya Dryomov]

On Tue, Oct 18, 2016 at 12:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> When building with gcc-4.9 -Wmaybe-uninitialized, we get a bogus
> warning in rbd_watch_cb, as the variable is not used at all
> in the one case in which it is not initialized first:
>
> drivers/block/rbd.c: In function ‘rbd_watch_cb’:
> drivers/block/rbd.c:3690:5: error: ‘struct_v’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/block/rbd.c:3759:5: note: ‘struct_v’ was declared here
>
> Later compiler versions fix this, but adding another initialization
> here is harmless and lets us build cleanly with 4.9 as well.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/block/rbd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index abb7162..4ab990b 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -3776,6 +3776,7 @@ static void rbd_watch_cb(void *arg, u64 notify_id, u64 cookie,
>         } else {
>                 /* legacy notification for header updates */
>                 notify_op = RBD_NOTIFY_OP_HEADER_UPDATE;
> +               struct_v = 0;
>                 len = 0;
>         }

It already got silenced by initializing at declaration in one of the
downstream trees, so I'd rather we do

@@ -3756,7 +3819,7 @@ static void rbd_watch_cb(void *arg, u64
notify_id, u64 cookie,
        struct rbd_device *rbd_dev = arg;
        void *p = data;
        void *const end = p + data_len;
-       u8 struct_v;
+       u8 struct_v = 0;
        u32 len;
        u32 notify_op;
        int ret;

to reduce the churn.

The "block" prefix is redundant and "rdb" should be "rbd" in the subject.

Thanks,

                Ilya

Re: [PATCH 11/28] block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized

[Arnd Bergmann]

On Tuesday, October 18, 2016 11:57:33 AM CEST Ilya Dryomov wrote:
> It already got silenced by initializing at declaration in one of the
> downstream trees, so I'd rather we do
> 
> @@ -3756,7 +3819,7 @@ static void rbd_watch_cb(void *arg, u64
> notify_id, u64 cookie,
>         struct rbd_device *rbd_dev = arg;
>         void *p = data;
>         void *const end = p + data_len;
> -       u8 struct_v;
> +       u8 struct_v = 0;
>         u32 len;
>         u32 notify_op;
>         int ret;
> 
> to reduce the churn.

Fair enough. I try to avoid adding extraneous initializations like
this, but my suggested change is not all that different here,
except if ceph_start_decoding() got changed in a way that could
lead to another uninitialized use.

> The "block" prefix is redundant and "rdb" should be "rbd" in the subject.

Oops.

	Arnd

[PATCH 12/28] [media] rc: print correct variable for z8f0811

[Arnd Bergmann]

A recent rework accidentally left a debugging printk untouched
while changing the meaning of the variables, leading to an
uninitialized variable being printed:

drivers/media/i2c/ir-kbd-i2c.c: In function 'get_key_haup_common':
drivers/media/i2c/ir-kbd-i2c.c:62:2: error: 'toggle' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This prints the correct one instead, as we did before the patch.

Fixes: 00bb820755ed ("[media] rc: Hauppauge z8f0811 can decode RC6")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/i2c/ir-kbd-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index f95a6bc..cede397 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -118,7 +118,7 @@ static int get_key_haup_common(struct IR_i2c *ir, enum rc_type *protocol,
 			*protocol = RC_TYPE_RC6_MCE;
 			dev &= 0x7f;
 			dprintk(1, "ir hauppauge (rc6-mce): t%d vendor=%d dev=%d code=%d\n",
-						toggle, vendor, dev, code);
+						*ptoggle, vendor, dev, code);
 		} else {
 			*ptoggle = 0;
 			*protocol = RC_TYPE_RC6_6A_32;
-- 
2.9.0

[PATCH 13/28] [media] dib0700: fix uninitialized data on 'repeat' event

[Arnd Bergmann]

After a recent cleanup patch, "gcc -Wmaybe-uninitialized" reports a new
warning about an existing bug:

drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
drivers/media/usb/dvb-usb/dib0700_core.c:763:2: error: ‘protocol’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

It turns out that the "0 0 0 FF" sequence of input data has already
caused an uninitialized data use for the keycode variable, but that
was hidden with the 'uninitialized_var()' macro. Now, the protocol
is also uninitialized.

This changes the code to not report any key for this sequence, which
fixes both problems, and allows us to also remove the misleading
uninitialized_var() annotation.

It is possible that we should call rc_repeat() here, but I'm not
sure about that.

Fixes: 2ceeca0499d7 ("[media] rc: split nec protocol into its three variants")
Fixes: d3c501d1938c ("V4L/DVB: dib0700: Fix RC protocol logic to properly handle NEC/NECx and RC-5")
Cc: Sean Young <sean@mess.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/usb/dvb-usb/dib0700_core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index f319665..3678ebf 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -677,7 +677,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
 	struct dvb_usb_device *d = purb->context;
 	struct dib0700_rc_response *poll_reply;
 	enum rc_type protocol;
-	u32 uninitialized_var(keycode);
+	u32 keycode;
 	u8 toggle;
 
 	deb_info("%s()\n", __func__);
@@ -742,11 +742,10 @@ static void dib0700_rc_urb_completion(struct urb *purb)
 			protocol = RC_TYPE_NEC;
 		}
 
+		rc_keydown(d->rc_dev, protocol, keycode, toggle);
 		break;
 	default:
 		deb_data("RC5 protocol\n");
-		protocol = RC_TYPE_RC5;
-		toggle = poll_reply->report_id;
 		keycode = RC_SCANCODE_RC5(poll_reply->rc5.system, poll_reply->rc5.data);
 
 		if ((poll_reply->rc5.data ^ poll_reply->rc5.not_data) != 0xff) {
@@ -754,14 +753,13 @@ static void dib0700_rc_urb_completion(struct urb *purb)
 			err("key failed integrity check: %02x %02x %02x %02x",
 			    poll_reply->rc5.not_used, poll_reply->rc5.system,
 			    poll_reply->rc5.data, poll_reply->rc5.not_data);
-			goto resubmit;
+			break;
 		}
 
+		rc_keydown(d->rc_dev, RC_TYPE_RC5, keycode, poll_reply->report_id);
 		break;
 	}
 
-	rc_keydown(d->rc_dev, protocol, keycode, toggle);
-
 resubmit:
 	/* Clean the buffer before we requeue */
 	memset(purb->transfer_buffer, 0, RC_MSG_SIZE_V1_20);
-- 
2.9.0

[PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable

[Arnd Bergmann]

The newly added __sca3000_get_base_freq function handles all valid
modes of the SCA3000_REG_ADDR_MODE register, but gcc notices
that any other value (i.e. 0x00) causes the base_freq variable to
not get initialized:

drivers/staging/iio/accel/sca3000_core.c: In function 'sca3000_write_raw':
drivers/staging/iio/accel/sca3000_core.c:527:23: error: 'base_freq' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds explicit error handling for unexpected register values,
to ensure this cannot happen.

Fixes: e0f3fc9b47e6 ("iio: accel: sca3000_core: implemented IIO_CHAN_INFO_SAMP_FREQ")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Ico Doornekamp <ico@pruts.nl>
Cc: Jonathan Cameron <jic23@kernel.org>
---
I submitted this on Sept 22, and Jonathan said he applied it to his
'togreg' tree, but it hasn't appeared in linux-next yet, presumably
since this was not considered material for v4.9.

If we enable the warning again by default, we may want to have the
fix merged for v4.9 after all.

 drivers/staging/iio/accel/sca3000_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
index d626125..564b36d 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -468,6 +468,8 @@ static inline int __sca3000_get_base_freq(struct sca3000_state *st,
 	case SCA3000_MEAS_MODE_OP_2:
 		*base_freq = info->option_mode_2_freq;
 		break;
+	default:
+		ret = -EINVAL;
 	}
 error_ret:
 	return ret;
-- 
2.9.0

[PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable

[Arnd Bergmann]

The newly added __sca3000_get_base_freq function handles all valid
modes of the SCA3000_REG_ADDR_MODE register, but gcc notices
that any other value (i.e. 0x00) causes the base_freq variable to
not get initialized:

drivers/staging/iio/accel/sca3000_core.c: In function 'sca3000_write_raw':
drivers/staging/iio/accel/sca3000_core.c:527:23: error: 'base_freq' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds explicit error handling for unexpected register values,
to ensure this cannot happen.

Fixes: e0f3fc9b47e6 ("iio: accel: sca3000_core: implemented IIO_CHAN_INFO_SAMP_FREQ")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Ico Doornekamp <ico@pruts.nl>
Cc: Jonathan Cameron <jic23@kernel.org>
---
I submitted this on Sept 22, and Jonathan said he applied it to his
'togreg' tree, but it hasn't appeared in linux-next yet, presumably
since this was not considered material for v4.9.

If we enable the warning again by default, we may want to have the
fix merged for v4.9 after all.

 drivers/staging/iio/accel/sca3000_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
index d626125..564b36d 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -468,6 +468,8 @@ static inline int __sca3000_get_base_freq(struct sca3000_state *st,
 	case SCA3000_MEAS_MODE_OP_2:
 		*base_freq = info->option_mode_2_freq;
 		break;
+	default:
+		ret = -EINVAL;
 	}
 error_ret:
 	return ret;
-- 
2.9.0

Re: [PATCH 14/28] iio: accel: sca3000_core: avoid potentially uninitialized variable

[Jonathan Cameron]

On 17/10/16 23:13, Arnd Bergmann wrote:
> The newly added __sca3000_get_base_freq function handles all valid
> modes of the SCA3000_REG_ADDR_MODE register, but gcc notices
> that any other value (i.e. 0x00) causes the base_freq variable to
> not get initialized:
> 
> drivers/staging/iio/accel/sca3000_core.c: In function 'sca3000_write_raw':
> drivers/staging/iio/accel/sca3000_core.c:527:23: error: 'base_freq' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This adds explicit error handling for unexpected register values,
> to ensure this cannot happen.
> 
> Fixes: e0f3fc9b47e6 ("iio: accel: sca3000_core: implemented IIO_CHAN_INFO_SAMP_FREQ")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Ico Doornekamp <ico@pruts.nl>
> Cc: Jonathan Cameron <jic23@kernel.org>
> ---
> I submitted this on Sept 22, and Jonathan said he applied it to his
> 'togreg' tree, but it hasn't appeared in linux-next yet, presumably
> since this was not considered material for v4.9.
Yes, it's just gone to Greg near the start of a big pull request for
4.10.
> 
> If we enable the warning again by default, we may want to have the
> fix merged for v4.9 after all.
I have no particular objection to it being merged faster.

Given things have gone horribly wrong if the hardware returns a value
other than those handled, it is more in the category of a nice
tidy up than an true bug fix.  Mind you always nice to handle
possible broken hardware as cleanly as possible.

Still it's risk free and as I'm about to send Greg a pull request for
some fixes, I'll put this in there as well and it'll work it's way
towards mainline.

Jonathan
> 
>  drivers/staging/iio/accel/sca3000_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
> index d626125..564b36d 100644
> --- a/drivers/staging/iio/accel/sca3000_core.c
> +++ b/drivers/staging/iio/accel/sca3000_core.c
> @@ -468,6 +468,8 @@ static inline int __sca3000_get_base_freq(struct sca3000_state *st,
>  	case SCA3000_MEAS_MODE_OP_2:
>  		*base_freq = info->option_mode_2_freq;
>  		break;
> +	default:
> +		ret = -EINVAL;
>  	}
>  error_ret:
>  	return ret;
> 

[PATCH 15/28] crypto: aesni: avoid -Wmaybe-uninitialized warning

[Arnd Bergmann]

The rfc4106 encrypy/decrypt helper functions cause an annoying
false-positive warning in allmodconfig if we turn on
-Wmaybe-uninitialized warnings again:

arch/x86/crypto/aesni-intel_glue.c: In function ‘helper_rfc4106_decrypt’:
include/linux/scatterlist.h:67:31: warning: ‘dst_sg_walk.sg’ may be used uninitialized in this function [-Wmaybe-uninitialized]

The problem seems to be that the compiler doesn't track the state of the
'one_entry_in_sg' variable across the kernel_fpu_begin/kernel_fpu_end
section.

This reorganizes the code to avoid that variable and have the shared
code in a separate function to avoid some of the conditional branches.

The resulting functions are a bit longer but also slightly less complex,
leaving no room for speculation on the part of the compiler.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
The conversion is nontrivial, and I have only build-tested it, so this
could use a careful review and testing.
---
 arch/x86/crypto/aesni-intel_glue.c | 121 ++++++++++++++++++++++---------------
 1 file changed, 73 insertions(+), 48 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 0ab5ee1..054155b 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -269,6 +269,34 @@ static void (*aesni_gcm_dec_tfm)(void *ctx, u8 *out,
 			u8 *hash_subkey, const u8 *aad, unsigned long aad_len,
 			u8 *auth_tag, unsigned long auth_tag_len);
 
+static inline void aesni_do_gcm_enc_tfm(void *ctx, u8 *out,
+			const u8 *in, unsigned long plaintext_len, u8 *iv,
+			u8 *hash_subkey, const u8 *aad, unsigned long aad_len,
+			u8 *auth_tag, unsigned long auth_tag_len)
+{
+	kernel_fpu_begin();
+	aesni_gcm_enc_tfm(ctx, out, in, plaintext_len, iv, hash_subkey,
+			  aad, aad_len, auth_tag, auth_tag_len);
+	kernel_fpu_end();
+}
+
+static inline int aesni_do_gcm_dec_tfm(void *ctx, u8 *out,
+			const u8 *in, unsigned long ciphertext_len, u8 *iv,
+			u8 *hash_subkey, const u8 *aad, unsigned long aad_len,
+			u8 *auth_tag, unsigned long auth_tag_len)
+{
+	kernel_fpu_begin();
+	aesni_gcm_dec_tfm(ctx, out, in, ciphertext_len, iv, hash_subkey, aad,
+			  aad_len, auth_tag, auth_tag_len);
+	kernel_fpu_end();
+
+	/* Compare generated tag with passed in tag. */
+	if (crypto_memneq(in + ciphertext_len, auth_tag, auth_tag_len))
+		return -EBADMSG;
+
+	return 0;
+}
+
 static inline struct
 aesni_rfc4106_gcm_ctx *aesni_rfc4106_gcm_ctx_get(struct crypto_aead *tfm)
 {
@@ -879,7 +907,6 @@ static int rfc4106_set_authsize(struct crypto_aead *parent,
 
 static int helper_rfc4106_encrypt(struct aead_request *req)
 {
-	u8 one_entry_in_sg = 0;
 	u8 *src, *dst, *assoc;
 	__be32 counter = cpu_to_be32(1);
 	struct crypto_aead *tfm = crypto_aead_reqtfm(req);
@@ -908,7 +935,6 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 	    req->src->offset + req->src->length <= PAGE_SIZE &&
 	    sg_is_last(req->dst) &&
 	    req->dst->offset + req->dst->length <= PAGE_SIZE) {
-		one_entry_in_sg = 1;
 		scatterwalk_start(&src_sg_walk, req->src);
 		assoc = scatterwalk_map(&src_sg_walk);
 		src = assoc + req->assoclen;
@@ -916,7 +942,23 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 		if (unlikely(req->src != req->dst)) {
 			scatterwalk_start(&dst_sg_walk, req->dst);
 			dst = scatterwalk_map(&dst_sg_walk) + req->assoclen;
+
+			aesni_do_gcm_enc_tfm(aes_ctx, dst, src, req->cryptlen, iv,
+					     ctx->hash_subkey, assoc, req->assoclen - 8,
+					     dst + req->cryptlen, auth_tag_len);
+
+			scatterwalk_unmap(dst - req->assoclen);
+			scatterwalk_advance(&dst_sg_walk, req->dst->length);
+			scatterwalk_done(&dst_sg_walk, 1, 0);
+		} else {
+			aesni_do_gcm_enc_tfm(aes_ctx, dst, src, req->cryptlen, iv,
+					     ctx->hash_subkey, assoc, req->assoclen - 8,
+					     dst + req->cryptlen, auth_tag_len);
 		}
+
+		scatterwalk_unmap(assoc);
+		scatterwalk_advance(&src_sg_walk, req->src->length);
+		scatterwalk_done(&src_sg_walk, req->src == req->dst, 0);
 	} else {
 		/* Allocate memory for src, dst, assoc */
 		assoc = kmalloc(req->cryptlen + auth_tag_len + req->assoclen,
@@ -925,28 +967,14 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 			return -ENOMEM;
 		scatterwalk_map_and_copy(assoc, req->src, 0,
 					 req->assoclen + req->cryptlen, 0);
-		src = assoc + req->assoclen;
-		dst = src;
-	}
+		dst = src = assoc + req->assoclen;
 
-	kernel_fpu_begin();
-	aesni_gcm_enc_tfm(aes_ctx, dst, src, req->cryptlen, iv,
-			  ctx->hash_subkey, assoc, req->assoclen - 8,
-			  dst + req->cryptlen, auth_tag_len);
-	kernel_fpu_end();
+		aesni_gcm_enc_tfm(aes_ctx, dst, src, req->cryptlen, iv,
+				  ctx->hash_subkey, assoc, req->assoclen - 8,
+				  dst + req->cryptlen, auth_tag_len);
 
-	/* The authTag (aka the Integrity Check Value) needs to be written
-	 * back to the packet. */
-	if (one_entry_in_sg) {
-		if (unlikely(req->src != req->dst)) {
-			scatterwalk_unmap(dst - req->assoclen);
-			scatterwalk_advance(&dst_sg_walk, req->dst->length);
-			scatterwalk_done(&dst_sg_walk, 1, 0);
-		}
-		scatterwalk_unmap(assoc);
-		scatterwalk_advance(&src_sg_walk, req->src->length);
-		scatterwalk_done(&src_sg_walk, req->src == req->dst, 0);
-	} else {
+		/* The authTag (aka the Integrity Check Value) needs to be written
+		 * back to the packet. */
 		scatterwalk_map_and_copy(dst, req->dst, req->assoclen,
 					 req->cryptlen + auth_tag_len, 1);
 		kfree(assoc);
@@ -956,7 +984,6 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
 
 static int helper_rfc4106_decrypt(struct aead_request *req)
 {
-	u8 one_entry_in_sg = 0;
 	u8 *src, *dst, *assoc;
 	unsigned long tempCipherLen = 0;
 	__be32 counter = cpu_to_be32(1);
@@ -990,47 +1017,45 @@ static int helper_rfc4106_decrypt(struct aead_request *req)
 	    req->src->offset + req->src->length <= PAGE_SIZE &&
 	    sg_is_last(req->dst) &&
 	    req->dst->offset + req->dst->length <= PAGE_SIZE) {
-		one_entry_in_sg = 1;
 		scatterwalk_start(&src_sg_walk, req->src);
 		assoc = scatterwalk_map(&src_sg_walk);
 		src = assoc + req->assoclen;
-		dst = src;
 		if (unlikely(req->src != req->dst)) {
 			scatterwalk_start(&dst_sg_walk, req->dst);
 			dst = scatterwalk_map(&dst_sg_walk) + req->assoclen;
-		}
-
-	} else {
-		/* Allocate memory for src, dst, assoc */
-		assoc = kmalloc(req->cryptlen + req->assoclen, GFP_ATOMIC);
-		if (!assoc)
-			return -ENOMEM;
-		scatterwalk_map_and_copy(assoc, req->src, 0,
-					 req->assoclen + req->cryptlen, 0);
-		src = assoc + req->assoclen;
-		dst = src;
-	}
 
-	kernel_fpu_begin();
-	aesni_gcm_dec_tfm(aes_ctx, dst, src, tempCipherLen, iv,
-			  ctx->hash_subkey, assoc, req->assoclen - 8,
-			  authTag, auth_tag_len);
-	kernel_fpu_end();
-
-	/* Compare generated tag with passed in tag. */
-	retval = crypto_memneq(src + tempCipherLen, authTag, auth_tag_len) ?
-		-EBADMSG : 0;
+			retval = aesni_do_gcm_dec_tfm(aes_ctx, dst, src,
+					tempCipherLen, iv, ctx->hash_subkey,
+					assoc, req->assoclen - 8, authTag,
+					auth_tag_len);
 
-	if (one_entry_in_sg) {
-		if (unlikely(req->src != req->dst)) {
 			scatterwalk_unmap(dst - req->assoclen);
 			scatterwalk_advance(&dst_sg_walk, req->dst->length);
 			scatterwalk_done(&dst_sg_walk, 1, 0);
+		} else {
+			dst = src;
+			retval = aesni_do_gcm_dec_tfm(aes_ctx, dst, src,
+					tempCipherLen, iv, ctx->hash_subkey,
+					assoc, req->assoclen - 8, authTag,
+					auth_tag_len);
 		}
 		scatterwalk_unmap(assoc);
 		scatterwalk_advance(&src_sg_walk, req->src->length);
 		scatterwalk_done(&src_sg_walk, req->src == req->dst, 0);
 	} else {
+		/* Allocate memory for src, dst, assoc */
+		assoc = kmalloc(req->cryptlen + req->assoclen, GFP_ATOMIC);
+		if (!assoc)
+			return -ENOMEM;
+		scatterwalk_map_and_copy(assoc, req->src, 0,
+					 req->assoclen + req->cryptlen, 0);
+		dst = src = assoc + req->assoclen;
+
+		retval = aesni_do_gcm_dec_tfm(aes_ctx, dst, src, tempCipherLen,
+					      iv, ctx->hash_subkey, assoc,
+					      req->assoclen - 8, authTag,
+					      auth_tag_len);
+
 		scatterwalk_map_and_copy(dst, req->dst, req->assoclen,
 					 tempCipherLen, 1);
 		kfree(assoc);
-- 
2.9.0

[PATCH 16/28] pcmcia: fix return value of soc_pcmcia_regulator_set

[Arnd Bergmann]

The newly introduced soc_pcmcia_regulator_set() function sometimes returns
without setting its return code, as shown by this warning:

drivers/pcmcia/soc_common.c: In function 'soc_pcmcia_regulator_set':
drivers/pcmcia/soc_common.c:112:5: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This changes it to propagate the regulator_disable() result instead.

Fixes: ac61b6001a63 ("pcmcia: soc_common: add support for Vcc and Vpp regulators")
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/pcmcia/soc_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index 153f312..b6b316d 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -107,7 +107,7 @@ int soc_pcmcia_regulator_set(struct soc_pcmcia_socket *skt,
 
 		ret = regulator_enable(r->reg);
 	} else {
-		regulator_disable(r->reg);
+		ret = regulator_disable(r->reg);
 	}
 	if (ret == 0)
 		r->on = on;
-- 
2.9.0

Re: [PATCH 16/28] pcmcia: fix return value of soc_pcmcia_regulator_set

[Russell King - ARM Linux]

On Tue, Oct 18, 2016 at 12:13:37AM +0200, Arnd Bergmann wrote:
> The newly introduced soc_pcmcia_regulator_set() function sometimes returns
> without setting its return code, as shown by this warning:
> 
> drivers/pcmcia/soc_common.c: In function 'soc_pcmcia_regulator_set':
> drivers/pcmcia/soc_common.c:112:5: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This changes it to propagate the regulator_disable() result instead.

I guess this is the problem with the stupid patch which silences this
warning - I don't see this warning here.

Having this warning disabled means that _real_ coding errors end up
making their way into the kernel (yes, this should be an error, this
is not a warning, because the value of 'ret' is completely undefined,
and therefore the behaviour of the following code is undefined.)

With the warning silenced, it means that such errors are undetectable.
I knew nothing about this until I received this patch, and I always
check that the code I send is warning-free on the GCC I'm using.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Arnd Bergmann]

When we get a spurious interrupt in fsl_espi_irq, we end up
processing four uninitalized bytes of data, as shown in this
warning message:

   drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
   drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds another check so we skip the data in this case.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/spi/spi-fsl-espi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7451585..2c175b9 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
 
 		mspi->len -= rx_nr_bytes;
 
-		if (mspi->rx)
+		if (rx_nr_bytes && mspi->rx)
 			mspi->get_rx(rx_data, mspi);
 	}
 
-- 
2.9.0

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Tue, Oct 18, 2016 at 12:13:38AM +0200, Arnd Bergmann wrote:
> When we get a spurious interrupt in fsl_espi_irq, we end up
> processing four uninitalized bytes of data, as shown in this
> warning message:

This doesn't apply against current code, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Tue, Oct 18, 2016 at 12:13:38AM +0200, Arnd Bergmann wrote:
> When we get a spurious interrupt in fsl_espi_irq, we end up
> processing four uninitalized bytes of data, as shown in this
> warning message:

This doesn't apply against current code, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Heiner Kallweit]

Am 24.10.2016 um 19:27 schrieb Mark Brown:
> On Tue, Oct 18, 2016 at 12:13:38AM +0200, Arnd Bergmann wrote:
>> When we get a spurious interrupt in fsl_espi_irq, we end up
>> processing four uninitalized bytes of data, as shown in this
>> warning message:
> 
> This doesn't apply against current code, please check and resend.
> 
The not yet reviewed part of my patch series from Oct 2nd,
namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
from RX FIFO" replaces the code in question.
There's more to fix like removing polling from the ISR.
If you prefer to apply Arnd's fix first I'd rebase the open part
of the patch series and resend it.

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Heiner Kallweit]

Am 24.10.2016 um 19:27 schrieb Mark Brown:
> On Tue, Oct 18, 2016 at 12:13:38AM +0200, Arnd Bergmann wrote:
>> When we get a spurious interrupt in fsl_espi_irq, we end up
>> processing four uninitalized bytes of data, as shown in this
>> warning message:
> 
> This doesn't apply against current code, please check and resend.
> 
The not yet reviewed part of my patch series from Oct 2nd,
namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
from RX FIFO" replaces the code in question.
There's more to fix like removing polling from the ISR.
If you prefer to apply Arnd's fix first I'd rebase the open part
of the patch series and resend it.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On Mon, Oct 24, 2016 at 08:36:37PM +0200, Heiner Kallweit wrote:
> Am 24.10.2016 um 19:27 schrieb Mark Brown:

> > This doesn't apply against current code, please check and resend.

> The not yet reviewed part of my patch series from Oct 2nd,
> namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
> from RX FIFO" replaces the code in question.
> There's more to fix like removing polling from the ISR.
> If you prefer to apply Arnd's fix first I'd rebase the open part
> of the patch series and resend it.

If there are dependencies you should mention them when you resend (in
general you should always mention any unapplied or cross tree
dependencies when sending things).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Arnd Bergmann]

On Monday, October 24, 2016 7:45:43 PM CEST Mark Brown wrote:
> On Mon, Oct 24, 2016 at 08:36:37PM +0200, Heiner Kallweit wrote:
> > Am 24.10.2016 um 19:27 schrieb Mark Brown:
> 
> > > This doesn't apply against current code, please check and resend.
> 
> > The not yet reviewed part of my patch series from Oct 2nd,
> > namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
> > from RX FIFO" replaces the code in question.
> > There's more to fix like removing polling from the ISR.
> > If you prefer to apply Arnd's fix first I'd rebase the open part
> > of the patch series and resend it.
> 
> If there are dependencies you should mention them when you resend (in
> general you should always mention any unapplied or cross tree
> dependencies when sending things).

I think my patch (the version I sent) should ideally make it into
v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
autobuilder with the -Wmaybe-uninitialized warning added back, and
it's one of the actual bugs I found (though rather unlikely
to hit in practice).

Merging with Heiner's patches should be trivial, and I'm pretty
sure we want the patch either way. Not sure if we need a backport,
it was introduced earlier this year in commit 6319a68011b8
("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
I now found.

	Arnd

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Mon, Oct 24, 2016 at 10:37:53PM +0200, Arnd Bergmann wrote:

> I think my patch (the version I sent) should ideally make it into
> v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
> autobuilder with the -Wmaybe-uninitialized warning added back, and
> it's one of the actual bugs I found (though rather unlikely
> to hit in practice).

> Merging with Heiner's patches should be trivial, and I'm pretty
> sure we want the patch either way. Not sure if we need a backport,
> it was introduced earlier this year in commit 6319a68011b8
> ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
> I now found.

Sorry but I've lost track of which patches are being talked about here.
If there's stuff for v4.9 can you send me a version that applies on
Linus' tree and I'll merge that up into what's applied for -next?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Mon, Oct 24, 2016 at 10:37:53PM +0200, Arnd Bergmann wrote:

> I think my patch (the version I sent) should ideally make it into
> v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
> autobuilder with the -Wmaybe-uninitialized warning added back, and
> it's one of the actual bugs I found (though rather unlikely
> to hit in practice).

> Merging with Heiner's patches should be trivial, and I'm pretty
> sure we want the patch either way. Not sure if we need a backport,
> it was introduced earlier this year in commit 6319a68011b8
> ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
> I now found.

Sorry but I've lost track of which patches are being talked about here.
If there's stuff for v4.9 can you send me a version that applies on
Linus' tree and I'll merge that up into what's applied for -next?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Arnd Bergmann]

On Tuesday, October 25, 2016 8:13:09 PM CEST Mark Brown wrote:
> 
> Not enough information to check signature validity.	Show Details
> On Mon, Oct 24, 2016 at 10:37:53PM +0200, Arnd Bergmann wrote:
> 
> > I think my patch (the version I sent) should ideally make it into
> > v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
> > autobuilder with the -Wmaybe-uninitialized warning added back, and
> > it's one of the actual bugs I found (though rather unlikely
> > to hit in practice).
> 
> > Merging with Heiner's patches should be trivial, and I'm pretty
> > sure we want the patch either way. Not sure if we need a backport,
> > it was introduced earlier this year in commit 6319a68011b8
> > ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
> > I now found.
> 
> Sorry but I've lost track of which patches are being talked about here.
> If there's stuff for v4.9 can you send me a version that applies on
> Linus' tree and I'll merge that up into what's applied for -next?
> 

Done.

	Arnd

Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree

[Mark Brown]

The patch

   spi: fsl-espi: avoid processing uninitalized data on error

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 5c0ba57744b1422d528f19430dd66d6803cea86f Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 25 Oct 2016 22:57:10 +0200
Subject: [PATCH] spi: fsl-espi: avoid processing uninitalized data on error

When we get a spurious interrupt in fsl_espi_irq, we end up
processing four uninitalized bytes of data, as shown in this
warning message:

   drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
   drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds another check so we skip the data in this case.

Fixes: 6319a68011b8 ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/spi/spi-fsl-espi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7451585a080e..2c175b9495f7 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
 
 		mspi->len -= rx_nr_bytes;
 
-		if (mspi->rx)
+		if (rx_nr_bytes && mspi->rx)
 			mspi->get_rx(rx_data, mspi);
 	}
 
-- 
2.8.1

Merge problem: Re: Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree

[Heiner Kallweit]

Am 26.10.2016 um 12:15 schrieb Mark Brown:
> The patch
> 
>    spi: fsl-espi: avoid processing uninitalized data on error
> 
> has been applied to the spi tree at
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 
> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.  
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark
> 
>>From 5c0ba57744b1422d528f19430dd66d6803cea86f Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 25 Oct 2016 22:57:10 +0200
> Subject: [PATCH] spi: fsl-espi: avoid processing uninitalized data on error
> 
> When we get a spurious interrupt in fsl_espi_irq, we end up
> processing four uninitalized bytes of data, as shown in this
> warning message:
> 
>    drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
>    drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This adds another check so we skip the data in this case.
> 
> Fixes: 6319a68011b8 ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  drivers/spi/spi-fsl-espi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
> index 7451585a080e..2c175b9495f7 100644
> --- a/drivers/spi/spi-fsl-espi.c
> +++ b/drivers/spi/spi-fsl-espi.c
> @@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
>  
>  		mspi->len -= rx_nr_bytes;
>  
> -		if (mspi->rx)
> +		if (rx_nr_bytes && mspi->rx)
>  			mspi->get_rx(rx_data, mspi);
>  	}
>  
> 
There seems to be a merge problem. Before the relevant code was:
(changed in recent commit "spi: fsl-espi: fix handling of word
sizes other than 8 bit")

if (mspi->rx) {
	*(u32 *)mspi->rx = rx_data;
	mspi->rx += 4;
}

Now it's:

if (rx_nr_bytes && mspi->rx) {
	mspi->get_rx(rx_data, mspi);
	mspi->rx += 4;
}

Instead it should be:

if (rx_nr_bytes && mspi->rx) {
	*(u32 *)mspi->rx = rx_data;
	mspi->rx += 4;
}

Re: Merge problem: Re: Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

On Wed, Oct 26, 2016 at 08:11:28PM +0200, Heiner Kallweit wrote:

> Instead it should be:
> 
> if (rx_nr_bytes && mspi->rx) {
> 	*(u32 *)mspi->rx = rx_data;
> 	mspi->rx += 4;
> }

Please send a patch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Merge problem: Re: Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

On Wed, Oct 26, 2016 at 08:11:28PM +0200, Heiner Kallweit wrote:

> Instead it should be:
> 
> if (rx_nr_bytes && mspi->rx) {
> 	*(u32 *)mspi->rx = rx_data;
> 	mspi->rx += 4;
> }

Please send a patch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank

[Arnd Bergmann]

gcc warns about the timestamp in drm_wait_vblank being possibly
used without an initialization:

drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This can happen if drm_vblank_count_and_time() returns 0 in its
error path. To sanitize the error case, I'm changing that function
to return a zero timestamp when it fails.

Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
First submitted in January 2016, second submission in February,
the patch is still required.

 drivers/gpu/drm/drm_irq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b969a64..48a6167 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
 	u32 vblank_count;
 	unsigned int seq;
 
-	if (WARN_ON(pipe >= dev->num_crtcs))
+	if (WARN_ON(pipe >= dev->num_crtcs)) {
+		*vblanktime = (struct timeval) { 0 };
 		return 0;
+	}
 
 	do {
 		seq = read_seqbegin(&vblank->seqlock);
-- 
2.9.0

[PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank

[Arnd Bergmann]

gcc warns about the timestamp in drm_wait_vblank being possibly
used without an initialization:

drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This can happen if drm_vblank_count_and_time() returns 0 in its
error path. To sanitize the error case, I'm changing that function
to return a zero timestamp when it fails.

Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
First submitted in January 2016, second submission in February,
the patch is still required.

 drivers/gpu/drm/drm_irq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index b969a64..48a6167 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
 	u32 vblank_count;
 	unsigned int seq;
 
-	if (WARN_ON(pipe >= dev->num_crtcs))
+	if (WARN_ON(pipe >= dev->num_crtcs)) {
+		*vblanktime = (struct timeval) { 0 };
 		return 0;
+	}
 
 	do {
 		seq = read_seqbegin(&vblank->seqlock);
-- 
2.9.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank

[Mario Kleiner]

On 10/18/2016 12:13 AM, Arnd Bergmann wrote:
> gcc warns about the timestamp in drm_wait_vblank being possibly
> used without an initialization:
>
> drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
> drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
> drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This can happen if drm_vblank_count_and_time() returns 0 in its
> error path. To sanitize the error case, I'm changing that function
> to return a zero timestamp when it fails.
>
> Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> First submitted in January 2016, second submission in February,
> the patch is still required.
>
>  drivers/gpu/drm/drm_irq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index b969a64..48a6167 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
>  	u32 vblank_count;
>  	unsigned int seq;
>
> -	if (WARN_ON(pipe >= dev->num_crtcs))
> +	if (WARN_ON(pipe >= dev->num_crtcs)) {
> +		*vblanktime = (struct timeval) { 0 };
>  		return 0;
> +	}
>
>  	do {
>  		seq = read_seqbegin(&vblank->seqlock);
>

Looks good to me.

Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>

-mario

Re: [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank

[Mario Kleiner]

On 10/18/2016 12:13 AM, Arnd Bergmann wrote:
> gcc warns about the timestamp in drm_wait_vblank being possibly
> used without an initialization:
>
> drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
> drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
> drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This can happen if drm_vblank_count_and_time() returns 0 in its
> error path. To sanitize the error case, I'm changing that function
> to return a zero timestamp when it fails.
>
> Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> First submitted in January 2016, second submission in February,
> the patch is still required.
>
>  drivers/gpu/drm/drm_irq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index b969a64..48a6167 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
>  	u32 vblank_count;
>  	unsigned int seq;
>
> -	if (WARN_ON(pipe >= dev->num_crtcs))
> +	if (WARN_ON(pipe >= dev->num_crtcs)) {
> +		*vblanktime = (struct timeval) { 0 };
>  		return 0;
> +	}
>
>  	do {
>  		seq = read_seqbegin(&vblank->seqlock);
>

Looks good to me.

Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>

-mario
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank

[Daniel Vetter]

On Tue, Oct 18, 2016 at 01:47:24AM +0200, Mario Kleiner wrote:
> On 10/18/2016 12:13 AM, Arnd Bergmann wrote:
> > gcc warns about the timestamp in drm_wait_vblank being possibly
> > used without an initialization:
> > 
> > drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
> > drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
> > drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 
> > This can happen if drm_vblank_count_and_time() returns 0 in its
> > error path. To sanitize the error case, I'm changing that function
> > to return a zero timestamp when it fails.
> > 
> > Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
> > Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > First submitted in January 2016, second submission in February,
> > the patch is still required.

Hm, sorry I missed that.

> >  drivers/gpu/drm/drm_irq.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index b969a64..48a6167 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
> >  	u32 vblank_count;
> >  	unsigned int seq;
> > 
> > -	if (WARN_ON(pipe >= dev->num_crtcs))
> > +	if (WARN_ON(pipe >= dev->num_crtcs)) {
> > +		*vblanktime = (struct timeval) { 0 };
> >  		return 0;
> > +	}
> > 
> >  	do {
> >  		seq = read_seqbegin(&vblank->seqlock);
> > 
> 
> Looks good to me.
> 
> Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Applied to drm-misc, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 18/28] drm: avoid uninitialized timestamp use in wait_vblank

[Daniel Vetter]

On Tue, Oct 18, 2016 at 01:47:24AM +0200, Mario Kleiner wrote:
> On 10/18/2016 12:13 AM, Arnd Bergmann wrote:
> > gcc warns about the timestamp in drm_wait_vblank being possibly
> > used without an initialization:
> > 
> > drivers/gpu/drm/drm_irq.c: In function 'drm_crtc_send_vblank_event':
> > drivers/gpu/drm/drm_irq.c:992:24: error: 'now.tv_usec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > drivers/gpu/drm/drm_irq.c:1069:17: note: 'now.tv_usec' was declared here
> > drivers/gpu/drm/drm_irq.c:991:23: error: 'now.tv_sec' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 
> > This can happen if drm_vblank_count_and_time() returns 0 in its
> > error path. To sanitize the error case, I'm changing that function
> > to return a zero timestamp when it fails.
> > 
> > Fixes: e6ae8687a87b ("drm: idiot-proof vblank")
> > Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > First submitted in January 2016, second submission in February,
> > the patch is still required.

Hm, sorry I missed that.

> >  drivers/gpu/drm/drm_irq.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index b969a64..48a6167 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -952,8 +952,10 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe,
> >  	u32 vblank_count;
> >  	unsigned int seq;
> > 
> > -	if (WARN_ON(pipe >= dev->num_crtcs))
> > +	if (WARN_ON(pipe >= dev->num_crtcs)) {
> > +		*vblanktime = (struct timeval) { 0 };
> >  		return 0;
> > +	}
> > 
> >  	do {
> >  		seq = read_seqbegin(&vblank->seqlock);
> > 
> 
> Looks good to me.
> 
> Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Applied to drm-misc, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Arnd Bergmann]

A bugfix added a sanity check around the assignment and use of the
'is_11d' variable, which looks correct to me, but as the function is
rather complex already, this confuses the compiler to the point where
it can no longer figure out if the variable is always initialized
correctly:

brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds an initialization for the newly introduced case in which
the variable should not really be used, in order to make the warning
go away.

Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
Cc: Hante Meuleman <hante.meuleman@broadcom.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b777e1b..78d9966 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4516,7 +4516,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 	/* store current 11d setting */
 	if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
 				  &ifp->vif->is_11d)) {
-		supports_11d = false;
+		is_11d = supports_11d = false;
 	} else {
 		country_ie = brcmf_parse_tlvs((u8 *)settings->beacon.tail,
 					      settings->beacon.tail_len,
-- 
2.9.0

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann <arnd@arndb.de> writes:

> A bugfix added a sanity check around the assignment and use of the
> 'is_11d' variable, which looks correct to me, but as the function is
> rather complex already, this confuses the compiler to the point where
> it can no longer figure out if the variable is always initialized
> correctly:
>
> brcm80211/brcmfmac/cfg80211.c: In function =E2=80=98brcmf_cfg80211_start_=
ap=E2=80=99:
> brcm80211/brcmfmac/cfg80211.c:4586:10: error: =E2=80=98is_11d=E2=80=99 ma=
y be used uninitialized in this function [-Werror=3Dmaybe-uninitialized]
>
> This adds an initialization for the newly introduced case in which
> the variable should not really be used, in order to make the warning
> go away.
>
> Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> Cc: Hante Meuleman <hante.meuleman@broadcom.com>
> Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Via which tree are you planning to submit this? Should I take it?

--=20
Kalle Valo

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann <arnd@arndb.de> writes:

> A bugfix added a sanity check around the assignment and use of the
> 'is_11d' variable, which looks correct to me, but as the function is
> rather complex already, this confuses the compiler to the point where
> it can no longer figure out if the variable is always initialized
> correctly:
>
> brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This adds an initialization for the newly introduced case in which
> the variable should not really be used, in order to make the warning
> go away.
>
> Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> Cc: Hante Meuleman <hante.meuleman@broadcom.com>
> Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Via which tree are you planning to submit this? Should I take it?

-- 
Kalle Valo

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Arnd Bergmann]

On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> 
> > A bugfix added a sanity check around the assignment and use of the
> > 'is_11d' variable, which looks correct to me, but as the function is
> > rather complex already, this confuses the compiler to the point where
> > it can no longer figure out if the variable is always initialized
> > correctly:
> >
> > brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > This adds an initialization for the newly introduced case in which
> > the variable should not really be used, in order to make the warning
> > go away.
> >
> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> > Cc: Hante Meuleman <hante.meuleman@broadcom.com>
> > Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> > Cc: Kalle Valo <kvalo@codeaurora.org>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Via which tree are you planning to submit this? Should I take it?

I'd prefer if you can take it and forward it along with your other
bugfixes. I'll try to take care of the ones that nobody else
picked up.

	Arnd

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann <arnd@arndb.de> writes:

> On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
>> Arnd Bergmann <arnd@arndb.de> writes:
>>=20
>> > A bugfix added a sanity check around the assignment and use of the
>> > 'is_11d' variable, which looks correct to me, but as the function is
>> > rather complex already, this confuses the compiler to the point where
>> > it can no longer figure out if the variable is always initialized
>> > correctly:
>> >
>> > brcm80211/brcmfmac/cfg80211.c: In function =E2=80=98brcmf_cfg80211_sta=
rt_ap=E2=80=99:
>> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: =E2=80=98is_11d=E2=80=99=
 may be used uninitialized in this function [-Werror=3Dmaybe-uninitialized]
>> >
>> > This adds an initialization for the newly introduced case in which
>> > the variable should not really be used, in order to make the warning
>> > go away.
>> >
>> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
>> > Cc: Hante Meuleman <hante.meuleman@broadcom.com>
>> > Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
>> > Cc: Kalle Valo <kvalo@codeaurora.org>
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>=20
>> Via which tree are you planning to submit this? Should I take it?
>
> I'd prefer if you can take it and forward it along with your other
> bugfixes. I'll try to take care of the ones that nobody else
> picked up.

Ok, I'll take it. I'm planning to push this to 4.9.

--=20
Kalle Valo

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:

> On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
>> Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:
>> 
>> > A bugfix added a sanity check around the assignment and use of the
>> > 'is_11d' variable, which looks correct to me, but as the function is
>> > rather complex already, this confuses the compiler to the point where
>> > it can no longer figure out if the variable is always initialized
>> > correctly:
>> >
>> > brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
>> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> >
>> > This adds an initialization for the newly introduced case in which
>> > the variable should not really be used, in order to make the warning
>> > go away.
>> >
>> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
>> > Cc: Hante Meuleman <hante.meuleman-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> > Cc: Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> > Cc: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> > Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>> 
>> Via which tree are you planning to submit this? Should I take it?
>
> I'd prefer if you can take it and forward it along with your other
> bugfixes. I'll try to take care of the ones that nobody else
> picked up.

Ok, I'll take it. I'm planning to push this to 4.9.

-- 
Kalle Valo

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann <arnd@arndb.de> writes:

> On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
>> Arnd Bergmann <arnd@arndb.de> writes:
>> 
>> > A bugfix added a sanity check around the assignment and use of the
>> > 'is_11d' variable, which looks correct to me, but as the function is
>> > rather complex already, this confuses the compiler to the point where
>> > it can no longer figure out if the variable is always initialized
>> > correctly:
>> >
>> > brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
>> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> >
>> > This adds an initialization for the newly introduced case in which
>> > the variable should not really be used, in order to make the warning
>> > go away.
>> >
>> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
>> > Cc: Hante Meuleman <hante.meuleman@broadcom.com>
>> > Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
>> > Cc: Kalle Valo <kvalo@codeaurora.org>
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> 
>> Via which tree are you planning to submit this? Should I take it?
>
> I'd prefer if you can take it and forward it along with your other
> bugfixes. I'll try to take care of the ones that nobody else
> picked up.

Ok, I'll take it. I'm planning to push this to 4.9.

-- 
Kalle Valo

Re: [19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann <arnd@arndb.de> wrote:
> A bugfix added a sanity check around the assignment and use of the
> 'is_11d' variable, which looks correct to me, but as the function is
> rather complex already, this confuses the compiler to the point where
> it can no longer figure out if the variable is always initialized
> correctly:
> 
> brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This adds an initialization for the newly introduced case in which
> the variable should not really be used, in order to make the warning
> go away.
> 
> Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> Cc: Hante Meuleman <hante.meuleman@broadcom.com>
> Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied to wireless-drivers.git, thanks.

d3532ea6ce4e brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

-- 
https://patchwork.kernel.org/patch/9380763/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> A bugfix added a sanity check around the assignment and use of the
> 'is_11d' variable, which looks correct to me, but as the function is
> rather complex already, this confuses the compiler to the point where
> it can no longer figure out if the variable is always initialized
> correctly:
> 
> brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This adds an initialization for the newly introduced case in which
> the variable should not really be used, in order to make the warning
> go away.
> 
> Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> Cc: Hante Meuleman <hante.meuleman-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Cc: Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Cc: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

Patch applied to wireless-drivers.git, thanks.

d3532ea6ce4e brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

-- 
https://patchwork.kernel.org/patch/9380763/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable

[Arnd Bergmann]

gcc found a reference to an uninitialized variable in the error handling
of bcm_enet_open, introduced by a recent cleanup:

drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]

This makes the use of that variable conditional, so we only reference it
here after it has been used before. Unlike my normal patches, I have not
build-tested this one, as I don't currently have mips test in my
randconfig setup.

Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
Cc: Philippe Reynes <tremyfr@gmail.com>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/bcm63xx_enet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index ae364c7..5370909 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1126,7 +1126,8 @@ static int bcm_enet_open(struct net_device *dev)
 	free_irq(dev->irq, dev);
 
 out_phy_disconnect:
-	phy_disconnect(phydev);
+	if (priv->has_phy)
+		phy_disconnect(phydev);
 
 	return ret;
 }
-- 
2.9.0

[PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable

[Arnd Bergmann]

gcc found a reference to an uninitialized variable in the error handling
of bcm_enet_open, introduced by a recent cleanup:

drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]

This makes the use of that variable conditional, so we only reference it
here after it has been used before. Unlike my normal patches, I have not
build-tested this one, as I don't currently have mips test in my
randconfig setup.

Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
Cc: Philippe Reynes <tremyfr@gmail.com>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/bcm63xx_enet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index ae364c7..5370909 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1126,7 +1126,8 @@ static int bcm_enet_open(struct net_device *dev)
 	free_irq(dev->irq, dev);
 
 out_phy_disconnect:
-	phy_disconnect(phydev);
+	if (priv->has_phy)
+		phy_disconnect(phydev);
 
 	return ret;
 }
-- 
2.9.0

Re: [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:08 +0200

> gcc found a reference to an uninitialized variable in the error handling
> of bcm_enet_open, introduced by a recent cleanup:
> 
> drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
> drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This makes the use of that variable conditional, so we only reference it
> here after it has been used before. Unlike my normal patches, I have not
> build-tested this one, as I don't currently have mips test in my
> randconfig setup.
> 
> Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
> Cc: Philippe Reynes <tremyfr@gmail.com>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

[PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:08 +0200

> gcc found a reference to an uninitialized variable in the error handling
> of bcm_enet_open, introduced by a recent cleanup:
> 
> drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
> drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This makes the use of that variable conditional, so we only reference it
> here after it has been used before. Unlike my normal patches, I have not
> build-tested this one, as I don't currently have mips test in my
> randconfig setup.
> 
> Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
> Cc: Philippe Reynes <tremyfr@gmail.com>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

[PATCH 21/28] net/hyperv: avoid uninitialized variable

[Arnd Bergmann]

The hdr_offset variable is only if we deal with a TCP or UDP packet,
but as the check surrounding its usage tests for skb_is_gso()
instead, the compiler has no idea if the variable is initialized
or not at that point:

drivers/net/hyperv/netvsc_drv.c: In function ‘netvsc_start_xmit’:
drivers/net/hyperv/netvsc_drv.c:494:42: error: ‘hdr_offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds an additional check for the transport type, which
tells the compiler that this path cannot happen. Since the
get_net_transport_info() function should always be inlined
here, I don't expect this to result in additional runtime
checks.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/hyperv/netvsc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f0919bd..5d6e75a 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -447,7 +447,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	 * Setup the sendside checksum offload only if this is not a
 	 * GSO packet.
 	 */
-	if (skb_is_gso(skb)) {
+	if ((net_trans_info & (INFO_TCP | INFO_UDP)) && skb_is_gso(skb)) {
 		struct ndis_tcp_lso_info *lso_info;
 
 		rndis_msg_size += NDIS_LSO_PPI_SIZE;
-- 
2.9.0

[PATCH 21/28] net/hyperv: avoid uninitialized variable

[Arnd Bergmann]

The hdr_offset variable is only if we deal with a TCP or UDP packet,
but as the check surrounding its usage tests for skb_is_gso()
instead, the compiler has no idea if the variable is initialized
or not at that point:

drivers/net/hyperv/netvsc_drv.c: In function ‘netvsc_start_xmit’:
drivers/net/hyperv/netvsc_drv.c:494:42: error: ‘hdr_offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This adds an additional check for the transport type, which
tells the compiler that this path cannot happen. Since the
get_net_transport_info() function should always be inlined
here, I don't expect this to result in additional runtime
checks.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/hyperv/netvsc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f0919bd..5d6e75a 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -447,7 +447,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	 * Setup the sendside checksum offload only if this is not a
 	 * GSO packet.
 	 */
-	if (skb_is_gso(skb)) {
+	if ((net_trans_info & (INFO_TCP | INFO_UDP)) && skb_is_gso(skb)) {
 		struct ndis_tcp_lso_info *lso_info;
 
 		rndis_msg_size += NDIS_LSO_PPI_SIZE;
-- 
2.9.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 21/28] net/hyperv: avoid uninitialized variable

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:09 +0200

> The hdr_offset variable is only if we deal with a TCP or UDP packet,
> but as the check surrounding its usage tests for skb_is_gso()
> instead, the compiler has no idea if the variable is initialized
> or not at that point:
> 
> drivers/net/hyperv/netvsc_drv.c: In function ‘netvsc_start_xmit’:
> drivers/net/hyperv/netvsc_drv.c:494:42: error: ‘hdr_offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This adds an additional check for the transport type, which
> tells the compiler that this path cannot happen. Since the
> get_net_transport_info() function should always be inlined
> here, I don't expect this to result in additional runtime
> checks.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

[PATCH 22/28] x86: apm: avoid uninitialized data

[Arnd Bergmann]

apm_bios_call() can fail, and return a status in its argument
structure. If that status however is zero during a call from
apm_get_power_status(), we end up using data that may have
never been set, as reported by "gcc -Wmaybe-uninitialized":

arch/x86/kernel/apm_32.c: In function ‘apm’:
arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here

This changes the function to return "APM_NO_ERROR" here, which
makes the code more robust to broken BIOS versions, and avoids
the warning.

Cc: x86@kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/kernel/apm_32.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index c7364bd..51287cd 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, u_short *bat, u_short *life)
 
 	if (apm_info.get_power_status_broken)
 		return APM_32_UNSUPPORTED;
-	if (apm_bios_call(&call))
+	if (apm_bios_call(&call)) {
+		if (!call.err)
+			return APM_NO_ERROR;
 		return call.err;
+	}
 	*status = call.ebx;
 	*bat = call.ecx;
 	if (apm_info.get_power_status_swabinminutes) {
-- 
2.9.0

Re: [PATCH 22/28] x86: apm: avoid uninitialized data

[Jiri Kosina]

On Tue, 18 Oct 2016, Arnd Bergmann wrote:

> apm_bios_call() can fail, and return a status in its argument
> structure. If that status however is zero during a call from
> apm_get_power_status(), we end up using data that may have
> never been set, as reported by "gcc -Wmaybe-uninitialized":
> 
> arch/x86/kernel/apm_32.c: In function ‘apm’:
> arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
> arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
> 
> This changes the function to return "APM_NO_ERROR" here, which
> makes the code more robust to broken BIOS versions, and avoids
> the warning.
> 
> Cc: x86@kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Makes sense.

Reviewed-by: Jiri Kosina <jkosina@suse.cz>

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 22/28] x86: apm: avoid uninitialized data

[Luis R. Rodriguez]

On Tue, Oct 18, 2016 at 12:16:10AM +0200, Arnd Bergmann wrote:
> apm_bios_call() can fail, and return a status in its argument
> structure. If that status however is zero during a call from
> apm_get_power_status(), we end up using data that may have
> never been set, as reported by "gcc -Wmaybe-uninitialized":

Userspace *may* already rely on this broken behavior for broken
BIOSes which may leave the return value as 0, ignoring that,
this change makes sense to me given that handling the error
would be better than relying any possible invalid data.

> arch/x86/kernel/apm_32.c: In function ‘apm’:
> arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
> arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
> 
> This changes the function to return "APM_NO_ERROR" here, which
> makes the code more robust to broken BIOS versions, and avoids
> the warning.
> 
> Cc: x86@kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org>

  Luis

> ---
>  arch/x86/kernel/apm_32.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
> index c7364bd..51287cd 100644
> --- a/arch/x86/kernel/apm_32.c
> +++ b/arch/x86/kernel/apm_32.c
> @@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, u_short *bat, u_short *life)
>  
>  	if (apm_info.get_power_status_broken)
>  		return APM_32_UNSUPPORTED;
> -	if (apm_bios_call(&call))
> +	if (apm_bios_call(&call)) {
> +		if (!call.err)
> +			return APM_NO_ERROR;
>  		return call.err;
> +	}
>  	*status = call.ebx;
>  	*bat = call.ecx;
>  	if (apm_info.get_power_status_swabinminutes) {
> -- 
> 2.9.0
> 
> 

-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg

[PATCH 23/28] x86: mark target address as output in 'insb' asm

[Arnd Bergmann]

The -Wmaybe-uninitialized warning triggers for one driver using the output
of the 'insb' I/O helper on x86:

drivers/net/wireless/wl3501_cs.c: In function ‘wl3501_mgmt_scan_confirm’:
drivers/net/wireless/wl3501_cs.c:665:9: error: ‘sig.status’ is used uninitialized in this function [-Werror=uninitialized]
drivers/net/wireless/wl3501_cs.c:668:12: error: ‘sig.cap_info’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

Apparently the assember constraints are slightly off here, as marking the
'addr' argument as a memory output seems appropriate here and gets rid
of the warning. For consistency I'm also adding it as input for outsb().

I'm not an x86 person and gcc inline assembly mystifies me all the time,
so please review this carefully and suggest a better way if this is not
how it should be done.

Cc: x86@kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/include/asm/io.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index de25aad..287234c 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -304,13 +304,13 @@ static inline unsigned type in##bwl##_p(int port)			\
 static inline void outs##bwl(int port, const void *addr, unsigned long count) \
 {									\
 	asm volatile("rep; outs" #bwl					\
-		     : "+S"(addr), "+c"(count) : "d"(port));		\
+		     : "+S"(addr), "+c"(count) : "d"(port), "m" (addr));\
 }									\
 									\
 static inline void ins##bwl(int port, void *addr, unsigned long count)	\
 {									\
 	asm volatile("rep; ins" #bwl					\
-		     : "+D"(addr), "+c"(count) : "d"(port));		\
+		     : "+D"(addr), "+c"(count), "=m" (addr) : "d"(port));\
 }
 
 BUILDIO(b, b, char)
-- 
2.9.0

[PATCH 24/28] x86: math-emu: possible uninitialized variable use

[Arnd Bergmann]

When building the kernel with "make EXTRA_CFLAGS=...", this overrides
the "PARANOID" preprocessor macro defined in arch/x86/math-emu/Makefile,
and we run into a build warning:

arch/x86/math-emu/reg_compare.c: In function ‘compare_i_st_st’:
arch/x86/math-emu/reg_compare.c:254:6: error: ‘f’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This fixes the implementation to work correctly even without the PARANOID
flag, and also fixes the Makefile to not use the EXTRA_CFLAGS variable
but instead use the ccflags-y variable in the Makefile that is meant
for this purpose.

Cc: x86@kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/math-emu/Makefile      |  4 ++--
 arch/x86/math-emu/reg_compare.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/math-emu/Makefile b/arch/x86/math-emu/Makefile
index 9b0c63b..1b2dac1 100644
--- a/arch/x86/math-emu/Makefile
+++ b/arch/x86/math-emu/Makefile
@@ -5,8 +5,8 @@
 #DEBUG	= -DDEBUGGING
 DEBUG	=
 PARANOID = -DPARANOID
-EXTRA_CFLAGS	:= $(PARANOID) $(DEBUG) -fno-builtin $(MATH_EMULATION)
-EXTRA_AFLAGS	:= $(PARANOID)
+ccflags-y += $(PARANOID) $(DEBUG) -fno-builtin $(MATH_EMULATION)
+asflags-y += $(PARANOID)
 
 # From 'C' language sources:
 C_OBJS =fpu_entry.o errors.o \
diff --git a/arch/x86/math-emu/reg_compare.c b/arch/x86/math-emu/reg_compare.c
index b77360f..19b33b50 100644
--- a/arch/x86/math-emu/reg_compare.c
+++ b/arch/x86/math-emu/reg_compare.c
@@ -168,7 +168,7 @@ static int compare(FPU_REG const *b, int tagb)
 /* This function requires that st(0) is not empty */
 int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 {
-	int f = 0, c;
+	int f, c;
 
 	c = compare(loaded_data, loaded_tag);
 
@@ -189,12 +189,12 @@ int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 		case COMP_No_Comp:
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#ifdef PARANOID
 		default:
+#ifdef PARANOID
 			EXCEPTION(EX_INTERNAL | 0x121);
+#endif /* PARANOID */
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#endif /* PARANOID */
 		}
 	setcc(f);
 	if (c & COMP_Denormal) {
@@ -205,7 +205,7 @@ int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 
 static int compare_st_st(int nr)
 {
-	int f = 0, c;
+	int f, c;
 	FPU_REG *st_ptr;
 
 	if (!NOT_EMPTY(0) || !NOT_EMPTY(nr)) {
@@ -235,12 +235,12 @@ static int compare_st_st(int nr)
 		case COMP_No_Comp:
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#ifdef PARANOID
 		default:
+#ifdef PARANOID
 			EXCEPTION(EX_INTERNAL | 0x122);
+#endif /* PARANOID */
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#endif /* PARANOID */
 		}
 	setcc(f);
 	if (c & COMP_Denormal) {
@@ -283,12 +283,12 @@ static int compare_i_st_st(int nr)
 	case COMP_No_Comp:
 		f = X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF;
 		break;
-#ifdef PARANOID
 	default:
+#ifdef PARANOID
 		EXCEPTION(EX_INTERNAL | 0x122);
+#endif /* PARANOID */
 		f = 0;
 		break;
-#endif /* PARANOID */
 	}
 	FPU_EFLAGS = (FPU_EFLAGS & ~(X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF)) | f;
 	if (c & COMP_Denormal) {
-- 
2.9.0

[PATCH 25/28] s390: pci: don't print uninitialized data for debugging

[Arnd Bergmann]

gcc correctly warns about an incorrect use of the 'pa' variable
in case we pass an empty scatterlist to __s390_dma_map_sg:

arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg':
arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds a bogus initialization to the function to sanitize
the debug output.  I would have preferred a solution without
the initialization, but I only got the report from the
kbuild bot after turning on the warning again, and didn't
manage to reproduce it myself.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/s390/pci/pci_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index 7350c8b..6b2f72f 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -423,7 +423,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	dma_addr_t dma_addr_base, dma_addr;
 	int flags = ZPCI_PTE_VALID;
 	struct scatterlist *s;
-	unsigned long pa;
+	unsigned long pa = 0;
 	int ret;
 
 	size = PAGE_ALIGN(size);
-- 
2.9.0

Re: [PATCH 25/28] s390: pci: don't print uninitialized data for debugging

[Martin Schwidefsky]

On Tue, 18 Oct 2016 00:16:13 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> gcc correctly warns about an incorrect use of the 'pa' variable
> in case we pass an empty scatterlist to __s390_dma_map_sg:
> 
> arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg':
> arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This adds a bogus initialization to the function to sanitize
> the debug output.  I would have preferred a solution without
> the initialization, but I only got the report from the
> kbuild bot after turning on the warning again, and didn't
> manage to reproduce it myself.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/s390/pci/pci_dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> index 7350c8b..6b2f72f 100644
> --- a/arch/s390/pci/pci_dma.c
> +++ b/arch/s390/pci/pci_dma.c
> @@ -423,7 +423,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
>  	dma_addr_t dma_addr_base, dma_addr;
>  	int flags = ZPCI_PTE_VALID;
>  	struct scatterlist *s;
> -	unsigned long pa;
> +	unsigned long pa = 0;
>  	int ret;
> 
>  	size = PAGE_ALIGN(size);

The compiler is right. pa is set in the for-loop. If "dma_addr < dma_addr_base + size"
is never true and __dma_purge_tlb() returns with an error, pa *is* uninitialized.
The fix seems sensible to me.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 25/28] s390: pci: don't print uninitialized data for debugging

[Sebastian Ott]

On Tue, 18 Oct 2016, Martin Schwidefsky wrote:
> On Tue, 18 Oct 2016 00:16:13 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > gcc correctly warns about an incorrect use of the 'pa' variable
> > in case we pass an empty scatterlist to __s390_dma_map_sg:
> > 
> > arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg':
> > arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > 
> > This adds a bogus initialization to the function to sanitize
> > the debug output.  I would have preferred a solution without
> > the initialization, but I only got the report from the
> > kbuild bot after turning on the warning again, and didn't
> > manage to reproduce it myself.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/s390/pci/pci_dma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> > index 7350c8b..6b2f72f 100644
> > --- a/arch/s390/pci/pci_dma.c
> > +++ b/arch/s390/pci/pci_dma.c
> > @@ -423,7 +423,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
> >  	dma_addr_t dma_addr_base, dma_addr;
> >  	int flags = ZPCI_PTE_VALID;
> >  	struct scatterlist *s;
> > -	unsigned long pa;
> > +	unsigned long pa = 0;
> >  	int ret;
> > 
> >  	size = PAGE_ALIGN(size);
> 
> The compiler is right. pa is set in the for-loop. If "dma_addr < dma_addr_base + size"
> is never true and __dma_purge_tlb() returns with an error, pa *is* uninitialized.
> The fix seems sensible to me.

Although that could only happen if map_sg is called with zero length sg
list entries I'm in favor getting rid of that warning.

Acked-by: Sebastian Ott <sebott@linux.vnet.ibm.com>

Regards,
Sebastian

[PATCH 26/28] nios2: fix timer initcall return value

[Arnd Bergmann]

When called more than twice, the nios2_time_init() function
return an uninitialized value, as detected by gcc -Wmaybe-uninitialized

arch/nios2/kernel/time.c: warning: 'ret' may be used uninitialized in this function

This makes it return '0' here, matching the comment above the
function.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/nios2/kernel/time.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/nios2/kernel/time.c b/arch/nios2/kernel/time.c
index d9563dd..746bf5c 100644
--- a/arch/nios2/kernel/time.c
+++ b/arch/nios2/kernel/time.c
@@ -324,6 +324,7 @@ static int __init nios2_time_init(struct device_node *timer)
 		ret = nios2_clocksource_init(timer);
 		break;
 	default:
+		ret = 0;
 		break;
 	}
 
-- 
2.9.0

Re: [PATCH 26/28] nios2: fix timer initcall return value

[Ley Foon Tan]

On Tue, Oct 18, 2016 at 6:16 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> When called more than twice, the nios2_time_init() function
> return an uninitialized value, as detected by gcc -Wmaybe-uninitialized
>
> arch/nios2/kernel/time.c: warning: 'ret' may be used uninitialized in this function
>
> This makes it return '0' here, matching the comment above the
> function.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/nios2/kernel/time.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/nios2/kernel/time.c b/arch/nios2/kernel/time.c
> index d9563dd..746bf5c 100644
> --- a/arch/nios2/kernel/time.c
> +++ b/arch/nios2/kernel/time.c
> @@ -324,6 +324,7 @@ static int __init nios2_time_init(struct device_node *timer)
>                 ret = nios2_clocksource_init(timer);
>                 break;
>         default:
> +               ret = 0;
>                 break;
>         }
Acked-by: Ley Foon Tan <lftan@altera.com>

[PATCH 27/28] rocker: fix maybe-uninitialized warning

[Arnd Bergmann]

In some rare configurations, we get a warning about the 'index' variable
being used without an initialization:

drivers/net/ethernet/rocker/rocker_ofdpa.c: In function ‘ofdpa_port_fib_ipv4.isra.16.constprop’:
drivers/net/ethernet/rocker/rocker_ofdpa.c:2425:92: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized]

This is a false positive, the logic is just a bit too complex for gcc
to follow here. Moving the intialization of 'index' a little further
down makes it clear to gcc that the function always returns an error
if it is not initialized.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/rocker/rocker_ofdpa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index 431a608..4ca4613 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -1493,8 +1493,6 @@ static int ofdpa_port_ipv4_nh(struct ofdpa_port *ofdpa_port,
 	spin_lock_irqsave(&ofdpa->neigh_tbl_lock, lock_flags);
 
 	found = ofdpa_neigh_tbl_find(ofdpa, ip_addr);
-	if (found)
-		*index = found->index;
 
 	updating = found && adding;
 	removing = found && !adding;
@@ -1508,9 +1506,11 @@ static int ofdpa_port_ipv4_nh(struct ofdpa_port *ofdpa_port,
 		resolved = false;
 	} else if (removing) {
 		ofdpa_neigh_del(trans, found);
+		*index = found->index;
 	} else if (updating) {
 		ofdpa_neigh_update(found, trans, NULL, false);
 		resolved = !is_zero_ether_addr(found->eth_dst);
+		*index = found->index;
 	} else {
 		err = -ENOENT;
 	}
-- 
2.9.0

Re: [PATCH 27/28] rocker: fix maybe-uninitialized warning

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:15 +0200

> In some rare configurations, we get a warning about the 'index' variable
> being used without an initialization:
> 
> drivers/net/ethernet/rocker/rocker_ofdpa.c: In function ‘ofdpa_port_fib_ipv4.isra.16.constprop’:
> drivers/net/ethernet/rocker/rocker_ofdpa.c:2425:92: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This is a false positive, the logic is just a bit too complex for gcc
> to follow here. Moving the intialization of 'index' a little further
> down makes it clear to gcc that the function always returns an error
> if it is not initialized.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

[PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning

[Arnd Bergmann]

Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].

Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE.  This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.

With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.

However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that
I had not addressed until then. This caused a lot of actual bugs to
get merged into mainline, and I sent several dozen patches for these
during the v4.9 development cycle. Most of these are actual bugs,
some are for correct code that is safe because it is only called
under external constraints that make it impossible to run into
the case that gcc sees, and in a few cases gcc is just stupid and
finds something that can obviously never happen.

I have now done a few thousand randconfig builds on x86 and collected
all patches that I needed to address every single warning I got
(I can provide the combined patch for the other warnings if anyone
is interested), so I hope we can get the warning back and let people
catch the actual bugs earlier.

Note that the majority of the patches I created are for the third kind
of problem (stupid false-positives), for one of two reasons:
- some of them only get triggered in certain combinations of config
  options, so we don't always run into them, and
- the actual bugs tend to get addressed much quicker as they also
  lead to incorrect runtime behavior.

These 27 patches address the warnings that either occur in one of the more
common configurations (defconfig, allmodconfig, or something built by the
kbuild robot or kernelci.org), or they are about a real bug. It would be
good to get these all into v4.9 if we want to turn on the warning again.
I have tested these extensively with gcc-4.9 and gcc-6 and done a bit
of testing with gcc-5, and all of these should now be fine. gcc-4.8
is much worse about the false-positive warnings and is also fairly old
now, so I'm leaving the warning disabled with that version. gcc-4.7 and
older don't understand the -Wno-maybe-uninitialized option and are not
affected by this patch either way.

I have another (smaller) series of patches for warnings that are both
harmless and not as easy to trigger, and I will send them for inclusion
in v4.10.

Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 Makefile               | 10 ++++++----
 arch/arc/Makefile      |  4 +++-
 scripts/Makefile.ubsan |  4 ++++
 3 files changed, 13 insertions(+), 5 deletions(-)

Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org

diff --git a/Makefile b/Makefile
index 512e47a..43cd3d9 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
 LDFLAGS_vmlinux =
-CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im  -Wno-maybe-uninitialized
 CFLAGS_KCOV	:= $(call cc-option,-fsanitize-coverage=trace-pc,)
 
 
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
 include arch/$(SRCARCH)/Makefile
 
 KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS	+= $(call cc-disable-warning,maybe-uninitialized,)
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 
 ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS	+= $(call cc-option,-fdata-sections,)
 endif
 
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS	+= -Os
+KBUILD_CFLAGS	+= -Os $(call cc-disable-warning,maybe-uninitialized,)
 else
 ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS	+= -O2
+KBUILD_CFLAGS	+= -O2 $(call cc-disable-warning,maybe-uninitialized,)
 else
 KBUILD_CFLAGS   += -O2
 endif
 endif
 
+KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
+			$(call cc-disable-warning,maybe-uninitialized,))
+
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index aa82d13..19cce22 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -71,7 +71,9 @@ cflags-$(CONFIG_ARC_DW2_UNWIND)		+= -fasynchronous-unwind-tables $(cfi)
 ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
 # Generic build system uses -O2, we want -O3
 # Note: No need to add to cflags-y as that happens anyways
-ARCH_CFLAGS += -O3
+#
+# Disable the false maybe-uninitialized warings gcc spits out at -O3
+ARCH_CFLAGS += -O3 $(call cc-disable-warning,maybe-uninitialized,)
 endif
 
 # small data is default for elf32 tool-chain. If not usable, disable it
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index dd779c4..3b1b138 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -17,4 +17,8 @@ endif
 ifdef CONFIG_UBSAN_NULL
       CFLAGS_UBSAN += $(call cc-option, -fsanitize=null)
 endif
+
+      # -fsanitize=* options makes GCC less smart than usual and
+      # increase number of 'maybe-uninitialized false-positives
+      CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
 endif
-- 
2.9.0

[PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning

[Arnd Bergmann]

Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].

Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE.  This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.

With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.

However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that
I had not addressed until then. This caused a lot of actual bugs to
get merged into mainline, and I sent several dozen patches for these
during the v4.9 development cycle. Most of these are actual bugs,
some are for correct code that is safe because it is only called
under external constraints that make it impossible to run into
the case that gcc sees, and in a few cases gcc is just stupid and
finds something that can obviously never happen.

I have now done a few thousand randconfig builds on x86 and collected
all patches that I needed to address every single warning I got
(I can provide the combined patch for the other warnings if anyone
is interested), so I hope we can get the warning back and let people
catch the actual bugs earlier.

Note that the majority of the patches I created are for the third kind
of problem (stupid false-positives), for one of two reasons:
- some of them only get triggered in certain combinations of config
  options, so we don't always run into them, and
- the actual bugs tend to get addressed much quicker as they also
  lead to incorrect runtime behavior.

These 27 patches address the warnings that either occur in one of the more
common configurations (defconfig, allmodconfig, or something built by the
kbuild robot or kernelci.org), or they are about a real bug. It would be
good to get these all into v4.9 if we want to turn on the warning again.
I have tested these extensively with gcc-4.9 and gcc-6 and done a bit
of testing with gcc-5, and all of these should now be fine. gcc-4.8
is much worse about the false-positive warnings and is also fairly old
now, so I'm leaving the warning disabled with that version. gcc-4.7 and
older don't understand the -Wno-maybe-uninitialized option and are not
affected by this patch either way.

I have another (smaller) series of patches for warnings that are both
harmless and not as easy to trigger, and I will send them for inclusion
in v4.10.

Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann <arnd at arndb.de>
---
 Makefile               | 10 ++++++----
 arch/arc/Makefile      |  4 +++-
 scripts/Makefile.ubsan |  4 ++++
 3 files changed, 13 insertions(+), 5 deletions(-)

Cc: x86 at kernel.org
Cc: linux-media at vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab at kernel.org>
Cc: Martin Schwidefsky <schwidefsky at de.ibm.com>
Cc: linux-s390 at vger.kernel.org
Cc: Ilya Dryomov <idryomov at gmail.com>
Cc: dri-devel at lists.freedesktop.org
Cc: linux-mtd at lists.infradead.org
Cc: Herbert Xu <herbert at gondor.apana.org.au>
Cc: linux-crypto at vger.kernel.org
Cc: "David S. Miller" <davem at davemloft.net>
Cc: netdev at vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
Cc: ceph-devel at vger.kernel.org
Cc: linux-f2fs-devel at lists.sourceforge.net
Cc: linux-ext4 at vger.kernel.org
Cc: netfilter-devel at vger.kernel.org

diff --git a/Makefile b/Makefile
index 512e47a..43cd3d9 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
 LDFLAGS_vmlinux =
-CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im  -Wno-maybe-uninitialized
 CFLAGS_KCOV	:= $(call cc-option,-fsanitize-coverage=trace-pc,)
 
 
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
 include arch/$(SRCARCH)/Makefile
 
 KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS	+= $(call cc-disable-warning,maybe-uninitialized,)
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 
 ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS	+= $(call cc-option,-fdata-sections,)
 endif
 
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS	+= -Os
+KBUILD_CFLAGS	+= -Os $(call cc-disable-warning,maybe-uninitialized,)
 else
 ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS	+= -O2
+KBUILD_CFLAGS	+= -O2 $(call cc-disable-warning,maybe-uninitialized,)
 else
 KBUILD_CFLAGS   += -O2
 endif
 endif
 
+KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
+			$(call cc-disable-warning,maybe-uninitialized,))
+
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index aa82d13..19cce22 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -71,7 +71,9 @@ cflags-$(CONFIG_ARC_DW2_UNWIND)		+= -fasynchronous-unwind-tables $(cfi)
 ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
 # Generic build system uses -O2, we want -O3
 # Note: No need to add to cflags-y as that happens anyways
-ARCH_CFLAGS += -O3
+#
+# Disable the false maybe-uninitialized warings gcc spits out at -O3
+ARCH_CFLAGS += -O3 $(call cc-disable-warning,maybe-uninitialized,)
 endif
 
 # small data is default for elf32 tool-chain. If not usable, disable it
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index dd779c4..3b1b138 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -17,4 +17,8 @@ endif
 ifdef CONFIG_UBSAN_NULL
       CFLAGS_UBSAN += $(call cc-option, -fsanitize=null)
 endif
+
+      # -fsanitize=* options makes GCC less smart than usual and
+      # increase number of 'maybe-uninitialized false-positives
+      CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
 endif
-- 
2.9.0

[PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning

[Arnd Bergmann]

Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].

Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE.  This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.

With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.

However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that
I had not addressed until then. This caused a lot of actual bugs to
get merged into mainline, and I sent several dozen patches for these
during the v4.9 development cycle. Most of these are actual bugs,
some are for correct code that is safe because it is only called
under external constraints that make it impossible to run into
the case that gcc sees, and in a few cases gcc is just stupid and
finds something that can obviously never happen.

I have now done a few thousand randconfig builds on x86 and collected
all patches that I needed to address every single warning I got
(I can provide the combined patch for the other warnings if anyone
is interested), so I hope we can get the warning back and let people
catch the actual bugs earlier.

Note that the majority of the patches I created are for the third kind
of problem (stupid false-positives), for one of two reasons:
- some of them only get triggered in certain combinations of config
  options, so we don't always run into them, and
- the actual bugs tend to get addressed much quicker as they also
  lead to incorrect runtime behavior.

These 27 patches address the warnings that either occur in one of the more
common configurations (defconfig, allmodconfig, or something built by the
kbuild robot or kernelci.org), or they are about a real bug. It would be
good to get these all into v4.9 if we want to turn on the warning again.
I have tested these extensively with gcc-4.9 and gcc-6 and done a bit
of testing with gcc-5, and all of these should now be fine. gcc-4.8
is much worse about the false-positive warnings and is also fairly old
now, so I'm leaving the warning disabled with that version. gcc-4.7 and
older don't understand the -Wno-maybe-uninitialized option and are not
affected by this patch either way.

I have another (smaller) series of patches for warnings that are both
harmless and not as easy to trigger, and I will send them for inclusion
in v4.10.

Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 Makefile               | 10 ++++++----
 arch/arc/Makefile      |  4 +++-
 scripts/Makefile.ubsan |  4 ++++
 3 files changed, 13 insertions(+), 5 deletions(-)

Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org

diff --git a/Makefile b/Makefile
index 512e47a..43cd3d9 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
 LDFLAGS_vmlinux =
-CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im  -Wno-maybe-uninitialized
 CFLAGS_KCOV	:= $(call cc-option,-fsanitize-coverage=trace-pc,)
 
 
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
 include arch/$(SRCARCH)/Makefile
 
 KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS	+= $(call cc-disable-warning,maybe-uninitialized,)
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 
 ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS	+= $(call cc-option,-fdata-sections,)
 endif
 
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS	+= -Os
+KBUILD_CFLAGS	+= -Os $(call cc-disable-warning,maybe-uninitialized,)
 else
 ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS	+= -O2
+KBUILD_CFLAGS	+= -O2 $(call cc-disable-warning,maybe-uninitialized,)
 else
 KBUILD_CFLAGS   += -O2
 endif
 endif
 
+KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
+			$(call cc-disable-warning,maybe-uninitialized,))
+
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index aa82d13..19cce22 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -71,7 +71,9 @@ cflags-$(CONFIG_ARC_DW2_UNWIND)		+= -fasynchronous-unwind-tables $(cfi)
 ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
 # Generic build system uses -O2, we want -O3
 # Note: No need to add to cflags-y as that happens anyways
-ARCH_CFLAGS += -O3
+#
+# Disable the false maybe-uninitialized warings gcc spits out at -O3
+ARCH_CFLAGS += -O3 $(call cc-disable-warning,maybe-uninitialized,)
 endif
 
 # small data is default for elf32 tool-chain. If not usable, disable it
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index dd779c4..3b1b138 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -17,4 +17,8 @@ endif
 ifdef CONFIG_UBSAN_NULL
       CFLAGS_UBSAN += $(call cc-option, -fsanitize=null)
 endif
+
+      # -fsanitize=* options makes GCC less smart than usual and
+      # increase number of 'maybe-uninitialized false-positives
+      CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
 endif
-- 
2.9.0

[PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning

[Arnd Bergmann]

Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].

Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE.  This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.

With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.

However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that
I had not addressed until then. This caused a lot of actual bugs to
get merged into mainline, and I sent several dozen patches for these
during the v4.9 development cycle. Most of these are actual bugs,
some are for correct code that is safe because it is only called
under external constraints that make it impossible to run into
the case that gcc sees, and in a few cases gcc is just stupid and
finds something that can obviously never happen.

I have now done a few thousand randconfig builds on x86 and collected
all patches that I needed to address every single warning I got
(I can provide the combined patch for the other warnings if anyone
is interested), so I hope we can get the warning back and let people
catch the actual bugs earlier.

Note that the majority of the patches I created are for the third kind
of problem (stupid false-positives), for one of two reasons:
- some of them only get triggered in certain combinations of config
  options, so we don't always run into them, and
- the actual bugs tend to get addressed much quicker as they also
  lead to incorrect runtime behavior.

These 27 patches address the warnings that either occur in one of the more
common configurations (defconfig, allmodconfig, or something built by the
kbuild robot or kernelci.org), or they are about a real bug. It would be
good to get these all into v4.9 if we want to turn on the warning again.
I have tested these extensively with gcc-4.9 and gcc-6 and done a bit
of testing with gcc-5, and all of these should now be fine. gcc-4.8
is much worse about the false-positive warnings and is also fairly old
now, so I'm leaving the warning disabled with that version. gcc-4.7 and
older don't understand the -Wno-maybe-uninitialized option and are not
affected by this patch either way.

I have another (smaller) series of patches for warnings that are both
harmless and not as easy to trigger, and I will send them for inclusion
in v4.10.

Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 Makefile               | 10 ++++++----
 arch/arc/Makefile      |  4 +++-
 scripts/Makefile.ubsan |  4 ++++
 3 files changed, 13 insertions(+), 5 deletions(-)

Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org

diff --git a/Makefile b/Makefile
index 512e47a..43cd3d9 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE  =
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
 LDFLAGS_vmlinux =
-CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV	= -fprofile-arcs -ftest-coverage -fno-tree-loop-im  -Wno-maybe-uninitialized
 CFLAGS_KCOV	:= $(call cc-option,-fsanitize-coverage=trace-pc,)
 
 
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
 include arch/$(SRCARCH)/Makefile
 
 KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS	+= $(call cc-disable-warning,maybe-uninitialized,)
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 
 ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS	+= $(call cc-option,-fdata-sections,)
 endif
 
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS	+= -Os
+KBUILD_CFLAGS	+= -Os $(call cc-disable-warning,maybe-uninitialized,)
 else
 ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS	+= -O2
+KBUILD_CFLAGS	+= -O2 $(call cc-disable-warning,maybe-uninitialized,)
 else
 KBUILD_CFLAGS   += -O2
 endif
 endif
 
+KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
+			$(call cc-disable-warning,maybe-uninitialized,))
+
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index aa82d13..19cce22 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -71,7 +71,9 @@ cflags-$(CONFIG_ARC_DW2_UNWIND)		+= -fasynchronous-unwind-tables $(cfi)
 ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
 # Generic build system uses -O2, we want -O3
 # Note: No need to add to cflags-y as that happens anyways
-ARCH_CFLAGS += -O3
+#
+# Disable the false maybe-uninitialized warings gcc spits out at -O3
+ARCH_CFLAGS += -O3 $(call cc-disable-warning,maybe-uninitialized,)
 endif
 
 # small data is default for elf32 tool-chain. If not usable, disable it
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index dd779c4..3b1b138 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -17,4 +17,8 @@ endif
 ifdef CONFIG_UBSAN_NULL
       CFLAGS_UBSAN += $(call cc-option, -fsanitize=null)
 endif
+
+      # -fsanitize=* options makes GCC less smart than usual and
+      # increase number of 'maybe-uninitialized false-positives
+      CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
 endif
-- 
2.9.0

Re: [PATCH 00/28] Reenable maybe-uninitialized warnings

[Christoph Hellwig]

On Tue, Oct 18, 2016 at 12:03:28AM +0200, Arnd Bergmann wrote:
> This is a set of patches that I hope to get into v4.9 in some form
> in order to turn on the -Wmaybe-uninitialized warnings again.

Hi Arnd,

I jsut complained to Geert that I was introducing way to many
bugs or pointless warnings for some compilers lately, but gcc didn't
warn me about them.  From a little research the lack of
-Wmaybe-uninitialized seems to be the reason for it, so I'm all
for re-enabling it.