> -----Original Message-----
> From: Peng, ZhihongX <[email protected]>
> Sent: Monday, October 18, 2021 1:55 PM
> To: Dumitrescu, Cristian <[email protected]>;
> [email protected]; Burakov, Anatoly
> <[email protected]>; Ananyev, Konstantin
> <[email protected]>; [email protected];
> Mcnamara, John <[email protected]>
> Cc: [email protected]; Lin, Xueqin <[email protected]>; [email protected]
> Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> 
> > -----Original Message-----
> > From: Dumitrescu, Cristian <[email protected]>
> > Sent: Monday, October 18, 2021 8:22 PM
> > To: Peng, ZhihongX <[email protected]>;
> > [email protected]; Burakov, Anatoly
> > <[email protected]>; Ananyev, Konstantin
> > <[email protected]>; [email protected];
> > Mcnamara, John <[email protected]>
> > Cc: [email protected]; Lin, Xueqin <[email protected]>; [email protected]
> > Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> >
> >
> >
> > > -----Original Message-----
> > > From: Peng, ZhihongX <[email protected]>
> > > Sent: Friday, October 15, 2021 4:11 PM
> > > To: [email protected]; Burakov, Anatoly
> > > <[email protected]>; Ananyev, Konstantin
> > > <[email protected]>; [email protected];
> > > Dumitrescu, Cristian <[email protected]>; Mcnamara, John
> > > <[email protected]>
> > > Cc: [email protected]; Lin, Xueqin <[email protected]>; Peng, ZhihongX
> > > <[email protected]>; [email protected]
> > > Subject: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> > >
> > > From: Zhihong Peng <[email protected]>
> > >
> > > The gcc will check code more stricter when ASan enabled.
> > > "Control reaches end of non-void function" error occurs here.
> > >
> > > Fixes: f38913b7fb8e ("pipeline: add meter array to SWX")
> > > Cc: [email protected]
> > >
> > > Signed-off-by: Xueqin Lin <[email protected]>
> > > Signed-off-by: Zhihong Peng <[email protected]>
> > > ---
> > > v7: no change
> > > v8: no change
> > > v9: Modify the submit log
> > > v10:no change
> > > ---
> > >  lib/pipeline/rte_swx_pipeline.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/pipeline/rte_swx_pipeline.c
> > > b/lib/pipeline/rte_swx_pipeline.c index 1cd09a4b44..0acd6c6752 100644
> > > --- a/lib/pipeline/rte_swx_pipeline.c
> > > +++ b/lib/pipeline/rte_swx_pipeline.c
> > > @@ -4642,7 +4642,7 @@ instr_meter_translate(struct rte_swx_pipeline
> *p,
> > >           return 0;
> > >   }
> > >
> > > - CHECK(0, EINVAL);
> > > + return -EINVAL;
> > >  }
> > >
> > >  static inline void
> > > @@ -5937,7 +5937,7 @@ instr_translate(struct rte_swx_pipeline *p,
> > >                                         instr,
> > >                                         data);
> > >
> > > - CHECK(0, EINVAL);
> > > + return -EINVAL;
> > >  }
> > >
> > >  static struct instruction_data *
> > > --
> > > 2.25.1
> >
> > NACK.
> >
> > This is a false issue, no bug is here. CHECK(0, EINVAL) translates to an
> > unconditional return -EINVAL.
> > Does this tool work correctly when macros are present? Maybe the tool
> > should parse the preprocessed C code as opposed to initial C code?
> 
> Yes, this is not a bug, it just solves the problem that cannot be passed after
> adding the asan compiler option.
> Only part of the macro reports errors, which may be caused by the tool itself,
> but this tool is part of gcc and clang, so we still have to make the code not
> report errors.
> > Regards,
> > Cristian

Hi Zhihong,

If this is not a bug in the pipeline library, why then does your patch has fix 
in the tile, has the Fixes label and CC-es [email protected]? Please remove and 
rephrase accordingly.

I agree this is not a bug, and based on your statements I understand this is a 
sort of issue or limitation with the tool . I would prefer we fix the tool 
rather than fixing correct code in order to please the tool. This is likely not 
going to be an isolated case, but a recurring issue.

Hence, I am reluctantly OK to ack this patch in order to allow the tool in 
(after the "fix" claim is removed), hopefully the tool will prove its benefit 
to DPDK.

Regards,
Cristian

Reply via email to