[PATCH] ioatdma: silence GCC warnings

[PATCH] ioatdma: silence GCC warnings

[Paul Bolle]

Building dma_v3.o triggers two GCC warnings:
    drivers/dma/ioat/dma_v3.c: In function ‘__ioat3_prep_pq16_lock’:
    drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
    drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]

These warnings are caused by pq16_set_src(). It uses "int idx" as an
index to an eight element array. Changing "idx" to unsigned int silences
these warnings. Apparently GCC can then determine that "idx" will never
be negative.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) Compile tested only.

1) These warning were introduced in v3.10-rc1. That must have been
through commit 7727eaa449 ("ioatdma: Adding support for 16 src PQ ops
and super extended descriptors").

 drivers/dma/ioat/dma_v3.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
index ca6ea9b..b5102da 100644
--- a/drivers/dma/ioat/dma_v3.c
+++ b/drivers/dma/ioat/dma_v3.c
@@ -251,7 +251,7 @@ static bool is_bwd_noraid(struct pci_dev *pdev)
 }
 
 static void pq16_set_src(struct ioat_raw_descriptor *desc[3],
-			dma_addr_t addr, u32 offset, u8 coef, int idx)
+			dma_addr_t addr, u32 offset, u8 coef, unsigned int idx)
 {
 	struct ioat_pq_descriptor *pq = (struct ioat_pq_descriptor *)desc[0];
 	struct ioat_pq16a_descriptor *pq16 =
@@ -1180,7 +1180,7 @@ __ioat3_prep_pq16_lock(struct dma_chan *c, enum sum_check_flags *result,
 	struct ioat_pq_descriptor *pq;
 	u32 offset = 0;
 	u8 op;
-	int i, s, idx, num_descs;
+	int i, idx, num_descs;
 
 	/* this function only handles src_cnt 9 - 16 */
 	BUG_ON(src_cnt < 9);
@@ -1206,6 +1206,7 @@ __ioat3_prep_pq16_lock(struct dma_chan *c, enum sum_check_flags *result,
 	do {
 		struct ioat_raw_descriptor *descs[4];
 		size_t xfer_size = min_t(size_t, len, 1 << ioat->xfercap_log);
+		unsigned int s;
 
 		desc = ioat2_get_ring_ent(ioat, idx + i);
 		pq = desc->pq;
-- 
1.8.1.4

Re: [PATCH] ioatdma: silence GCC warnings

[Vinod Koul]

On Mon, Jul 15, 2013 at 11:50:07AM +0200, Paul Bolle wrote:
> On Mon, 2013-06-17 at 12:35 +0200, Paul Bolle wrote:
> > Building dma_v3.o triggers two GCC warnings:
> >     drivers/dma/ioat/dma_v3.c: In function ‘__ioat3_prep_pq16_lock’:
> >     drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
> >     drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
> > 
> > These warnings are caused by pq16_set_src(). It uses "int idx" as an
> > index to an eight element array. Changing "idx" to unsigned int silences
> > these warnings. Apparently GCC can then determine that "idx" will never
> > be negative.
> > 
> > Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> 
> Identical warnings can still be seen in v3.11-rc1. Did anyone had a
> chance to look at this patch?
Looks okay to me. Dan do you want this to go thru my tree with you Ack of
course...

~Vinod
> 
> 
> Paul Bolle
> > ---
> > 0) Compile tested only.
> > 
> > 1) These warning were introduced in v3.10-rc1. That must have been
> > through commit 7727eaa449 ("ioatdma: Adding support for 16 src PQ ops
> > and super extended descriptors").
> > 
> >  drivers/dma/ioat/dma_v3.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
> > index ca6ea9b..b5102da 100644
> > --- a/drivers/dma/ioat/dma_v3.c
> > +++ b/drivers/dma/ioat/dma_v3.c
> > @@ -251,7 +251,7 @@ static bool is_bwd_noraid(struct pci_dev *pdev)
> >  }
> >  
> >  static void pq16_set_src(struct ioat_raw_descriptor *desc[3],
> > -			dma_addr_t addr, u32 offset, u8 coef, int idx)
> > +			dma_addr_t addr, u32 offset, u8 coef, unsigned int idx)
> >  {
> >  	struct ioat_pq_descriptor *pq = (struct ioat_pq_descriptor *)desc[0];
> >  	struct ioat_pq16a_descriptor *pq16 =
> > @@ -1180,7 +1180,7 @@ __ioat3_prep_pq16_lock(struct dma_chan *c, enum sum_check_flags *result,
> >  	struct ioat_pq_descriptor *pq;
> >  	u32 offset = 0;
> >  	u8 op;
> > -	int i, s, idx, num_descs;
> > +	int i, idx, num_descs;
> >  
> >  	/* this function only handles src_cnt 9 - 16 */
> >  	BUG_ON(src_cnt < 9);
> > @@ -1206,6 +1206,7 @@ __ioat3_prep_pq16_lock(struct dma_chan *c, enum sum_check_flags *result,
> >  	do {
> >  		struct ioat_raw_descriptor *descs[4];
> >  		size_t xfer_size = min_t(size_t, len, 1 << ioat->xfercap_log);
> > +		unsigned int s;
> >  
> >  		desc = ioat2_get_ring_ent(ioat, idx + i);
> >  		pq = desc->pq;
> 

-- 

Re: [PATCH] ioatdma: silence GCC warnings

[Paul Bolle]

On Mon, 2013-06-17 at 12:35 +0200, Paul Bolle wrote:
> Building dma_v3.o triggers two GCC warnings:
>     drivers/dma/ioat/dma_v3.c: In function ‘__ioat3_prep_pq16_lock’:
>     drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
>     drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
> 
> These warnings are caused by pq16_set_src(). It uses "int idx" as an
> index to an eight element array. Changing "idx" to unsigned int silences
> these warnings. Apparently GCC can then determine that "idx" will never
> be negative.
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>

Identical warnings can still be seen in v3.11-rc1. Did anyone had a
chance to look at this patch?


Paul Bolle
> ---
> 0) Compile tested only.
> 
> 1) These warning were introduced in v3.10-rc1. That must have been
> through commit 7727eaa449 ("ioatdma: Adding support for 16 src PQ ops
> and super extended descriptors").
> 
>  drivers/dma/ioat/dma_v3.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
> index ca6ea9b..b5102da 100644
> --- a/drivers/dma/ioat/dma_v3.c
> +++ b/drivers/dma/ioat/dma_v3.c
> @@ -251,7 +251,7 @@ static bool is_bwd_noraid(struct pci_dev *pdev)
>  }
>  
>  static void pq16_set_src(struct ioat_raw_descriptor *desc[3],
> -			dma_addr_t addr, u32 offset, u8 coef, int idx)
> +			dma_addr_t addr, u32 offset, u8 coef, unsigned int idx)
>  {
>  	struct ioat_pq_descriptor *pq = (struct ioat_pq_descriptor *)desc[0];
>  	struct ioat_pq16a_descriptor *pq16 =
> @@ -1180,7 +1180,7 @@ __ioat3_prep_pq16_lock(struct dma_chan *c, enum sum_check_flags *result,
>  	struct ioat_pq_descriptor *pq;
>  	u32 offset = 0;
>  	u8 op;
> -	int i, s, idx, num_descs;
> +	int i, idx, num_descs;
>  
>  	/* this function only handles src_cnt 9 - 16 */
>  	BUG_ON(src_cnt < 9);
> @@ -1206,6 +1206,7 @@ __ioat3_prep_pq16_lock(struct dma_chan *c, enum sum_check_flags *result,
>  	do {
>  		struct ioat_raw_descriptor *descs[4];
>  		size_t xfer_size = min_t(size_t, len, 1 << ioat->xfercap_log);
> +		unsigned int s;
>  
>  		desc = ioat2_get_ring_ent(ioat, idx + i);
>  		pq = desc->pq;

Re: [PATCH] ioatdma: silence GCC warnings

[Dan Williams]

On Mon, Jun 17, 2013 at 3:35 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> Building dma_v3.o triggers two GCC warnings:
>     drivers/dma/ioat/dma_v3.c: In function ‘__ioat3_prep_pq16_lock’:
>     drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
>     drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
>
> These warnings are caused by pq16_set_src(). It uses "int idx" as an
> index to an eight element array. Changing "idx" to unsigned int silences
> these warnings. Apparently GCC can then determine that "idx" will never
> be negative.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---

Seems this is fixed in gcc as of 4.7.2, but we can make things quieter
for build testers on older compilers.

> 0) Compile tested only.
>
> 1) These warning were introduced in v3.10-rc1. That must have been
> through commit 7727eaa449 ("ioatdma: Adding support for 16 src PQ ops
> and super extended descriptors").
>
>  drivers/dma/ioat/dma_v3.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
> index ca6ea9b..b5102da 100644
> --- a/drivers/dma/ioat/dma_v3.c
> +++ b/drivers/dma/ioat/dma_v3.c
> @@ -251,7 +251,7 @@ static bool is_bwd_noraid(struct pci_dev *pdev)
>  }
>
>  static void pq16_set_src(struct ioat_raw_descriptor *desc[3],
> -                       dma_addr_t addr, u32 offset, u8 coef, int idx)
> +                       dma_addr_t addr, u32 offset, u8 coef, unsigned int idx)

"unsigned int" --> "unsigned"

>  {
>         struct ioat_pq_descriptor *pq = (struct ioat_pq_descriptor *)desc[0];
>         struct ioat_pq16a_descriptor *pq16 =

Just do this hunk as the minimal one line change.

Re: [PATCH] ioatdma: silence GCC warnings

[Paul Bolle]

On Wed, 2013-07-17 at 15:39 -0700, Dan Williams wrote:
> On Mon, Jun 17, 2013 at 3:35 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> Seems this is fixed in gcc as of 4.7.2, but we can make things quieter
> for build testers on older compilers.

Well, I'm currently using
    gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8)

as shipped for Fedora 18.

> > diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
> > index ca6ea9b..b5102da 100644
> > --- a/drivers/dma/ioat/dma_v3.c
> > +++ b/drivers/dma/ioat/dma_v3.c
> > @@ -251,7 +251,7 @@ static bool is_bwd_noraid(struct pci_dev *pdev)
> >  }
> >
> >  static void pq16_set_src(struct ioat_raw_descriptor *desc[3],
> > -                       dma_addr_t addr, u32 offset, u8 coef, int idx)
> > +                       dma_addr_t addr, u32 offset, u8 coef, unsigned int idx)
> 
> "unsigned int" --> "unsigned"

Fine with me, but it does introduce a second form in this file (it
already uses "unsigned int" a few times).

> >  {
> >         struct ioat_pq_descriptor *pq = (struct ioat_pq_descriptor *)desc[0];
> >         struct ioat_pq16a_descriptor *pq16 =
> 
> Just do this hunk as the minimal one line change.

You're right. Just this hunk is enough. I'll try to send a v2 shortly.


Paul Bolle

[PATCH v2] ioatdma: silence GCC warnings

[Paul Bolle]

Building dma_v3.o triggers a GCC warning:
    drivers/dma/ioat/dma_v3.c: In function ‘__ioat3_prep_pq16_lock’:
    drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
    drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]

This warning is caused by pq16_set_src(). It uses "int idx" as an index
to an eight element array. Changing "idx" to "unsigned" silences this
warning. Apparently GCC can then determine that "idx" will never be
negative.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) v2: cut back to a one hunk change, as Dan suggested.

1) Still just compile tested.

 drivers/dma/ioat/dma_v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
index b642e03..2bb4601 100644
--- a/drivers/dma/ioat/dma_v3.c
+++ b/drivers/dma/ioat/dma_v3.c
@@ -251,7 +251,7 @@ static bool is_bwd_noraid(struct pci_dev *pdev)
 }
 
 static void pq16_set_src(struct ioat_raw_descriptor *desc[3],
-			dma_addr_t addr, u32 offset, u8 coef, int idx)
+			dma_addr_t addr, u32 offset, u8 coef, unsigned idx)
 {
 	struct ioat_pq_descriptor *pq = (struct ioat_pq_descriptor *)desc[0];
 	struct ioat_pq16a_descriptor *pq16 =
-- 
1.8.1.4

Re: [PATCH v2] ioatdma: silence GCC warnings

[Jiang, Dave]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1604 bytes --]

On Sat, 2013-07-20 at 20:05 +0200, Paul Bolle wrote:
> Building dma_v3.o triggers a GCC warning:
>     drivers/dma/ioat/dma_v3.c: In function ‘__ioat3_prep_pq16_lock’:
>     drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
>     drivers/dma/ioat/dma_v3.c:264:11: warning: array subscript is below array bounds [-Warray-bounds]
> 
> This warning is caused by pq16_set_src(). It uses "int idx" as an index
> to an eight element array. Changing "idx" to "unsigned" silences this
> warning. Apparently GCC can then determine that "idx" will never be
> negative.
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
> 0) v2: cut back to a one hunk change, as Dan suggested.
> 
> 1) Still just compile tested.
> 
>  drivers/dma/ioat/dma_v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
> index b642e03..2bb4601 100644
> --- a/drivers/dma/ioat/dma_v3.c
> +++ b/drivers/dma/ioat/dma_v3.c
> @@ -251,7 +251,7 @@ static bool is_bwd_noraid(struct pci_dev *pdev)
>  }
>  
>  static void pq16_set_src(struct ioat_raw_descriptor *desc[3],
> -			dma_addr_t addr, u32 offset, u8 coef, int idx)
> +			dma_addr_t addr, u32 offset, u8 coef, unsigned idx)
>  {
>  	struct ioat_pq_descriptor *pq = (struct ioat_pq_descriptor *)desc[0];
>  	struct ioat_pq16a_descriptor *pq16 =

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥