[v2] raw/ioat: fix parameter shadow warning
Checks
Commit Message
In the function __idxd_completed_ops() we have a parameter shadow warning
due to a local variable having the same name as one of the function
parameters. This issue is fixed by simply renaming the local variable.
This warning was discovered during an OVS build with DPDK 21.05-rc2. The
OVS build passes the -Wshadow flag by default, allowing the warning to be
seen.
Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")
Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
---
v2: add details of warning discovery
---
drivers/raw/ioat/rte_idxd_rawdev_fns.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Comments
On Mon, May 10, 2021 at 12:55:14PM +0000, Kevin Laatz wrote:
> In the function __idxd_completed_ops() we have a parameter shadow warning
> due to a local variable having the same name as one of the function
> parameters. This issue is fixed by simply renaming the local variable.
>
> This warning was discovered during an OVS build with DPDK 21.05-rc2. The
> OVS build passes the -Wshadow flag by default, allowing the warning to be
> seen.
>
> Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")
>
> Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
On Mon, May 10, 2021 at 2:55 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
>
> In the function __idxd_completed_ops() we have a parameter shadow warning
> due to a local variable having the same name as one of the function
> parameters. This issue is fixed by simply renaming the local variable.
>
> This warning was discovered during an OVS build with DPDK 21.05-rc2. The
> OVS build passes the -Wshadow flag by default, allowing the warning to be
> seen.
A bit confusing.
-Wshadow only affects OVS code and there is no code calling this in
the OVS master branch.
I did not see this issue while updating my dpdk-latest OVS branch and
running builds in GHA.
So I guess Sunil caught it with his patch:
https://patchwork.ozlabs.org/project/openvswitch/patch/20201023094845.35652-2-sunil.pai.g@intel.com/
On Mon, May 10, 2021 at 04:06:00PM +0200, David Marchand wrote:
> On Mon, May 10, 2021 at 2:55 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
> >
> > In the function __idxd_completed_ops() we have a parameter shadow warning
> > due to a local variable having the same name as one of the function
> > parameters. This issue is fixed by simply renaming the local variable.
> >
> > This warning was discovered during an OVS build with DPDK 21.05-rc2. The
> > OVS build passes the -Wshadow flag by default, allowing the warning to be
> > seen.
>
> A bit confusing.
> -Wshadow only affects OVS code and there is no code calling this in
> the OVS master branch.
>
> I did not see this issue while updating my dpdk-latest OVS branch and
> running builds in GHA.
> So I guess Sunil caught it with his patch:
> https://patchwork.ozlabs.org/project/openvswitch/patch/20201023094845.35652-2-sunil.pai.g@intel.com/
>
Yes, it was caught by Sunil in the course of his work.
Ideally, I think -Wshadow would be a good flag to add to our DPDK builds,
but it causes quite a number of errors right now to do so. Hopefully in a
future release.
/Bruce
10/05/2021 16:48, Bruce Richardson:
> On Mon, May 10, 2021 at 04:06:00PM +0200, David Marchand wrote:
> > On Mon, May 10, 2021 at 2:55 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
> > >
> > > In the function __idxd_completed_ops() we have a parameter shadow warning
> > > due to a local variable having the same name as one of the function
> > > parameters. This issue is fixed by simply renaming the local variable.
> > >
> > > This warning was discovered during an OVS build with DPDK 21.05-rc2. The
> > > OVS build passes the -Wshadow flag by default, allowing the warning to be
> > > seen.
> >
> > A bit confusing.
> > -Wshadow only affects OVS code and there is no code calling this in
> > the OVS master branch.
> >
> > I did not see this issue while updating my dpdk-latest OVS branch and
> > running builds in GHA.
> > So I guess Sunil caught it with his patch:
> > https://patchwork.ozlabs.org/project/openvswitch/patch/20201023094845.35652-2-sunil.pai.g@intel.com/
> >
> Yes, it was caught by Sunil in the course of his work.
So the commit message should be fixed please.
@@ -301,11 +301,11 @@ __idxd_completed_ops(int dev_id, uint8_t max_ops, uint32_t *status, uint8_t *num
uint16_t idx_to_chk = idxd->batch_idx_ring[idxd->batch_idx_read];
volatile struct rte_idxd_completion *comp_to_chk =
(struct rte_idxd_completion *)&idxd->desc_ring[idx_to_chk];
- uint8_t status = comp_to_chk->status;
- if (status == 0)
+ uint8_t batch_status = comp_to_chk->status;
+ if (batch_status == 0)
break;
comp_to_chk->status = 0;
- if (unlikely(status > 1)) {
+ if (unlikely(batch_status > 1)) {
/* error occurred somewhere in batch, start where last checked */
uint16_t desc_count = comp_to_chk->completed_size;
uint16_t batch_start = idxd->hdls_avail;