> -----Original Message-----
> From: Peng, ZhihongX <zhihongx.p...@intel.com>
> Sent: Monday, October 18, 2021 1:55 PM
> To: Dumitrescu, Cristian <cristian.dumitre...@intel.com>;
> david.march...@redhat.com; Burakov, Anatoly
> <anatoly.bura...@intel.com>; Ananyev, Konstantin
> <konstantin.anan...@intel.com>; step...@networkplumber.org;
> Mcnamara, John <john.mcnam...@intel.com>
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin....@intel.com>; sta...@dpdk.org
> Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> 
> > -----Original Message-----
> > From: Dumitrescu, Cristian <cristian.dumitre...@intel.com>
> > Sent: Monday, October 18, 2021 8:22 PM
> > To: Peng, ZhihongX <zhihongx.p...@intel.com>;
> > david.march...@redhat.com; Burakov, Anatoly
> > <anatoly.bura...@intel.com>; Ananyev, Konstantin
> > <konstantin.anan...@intel.com>; step...@networkplumber.org;
> > Mcnamara, John <john.mcnam...@intel.com>
> > Cc: dev@dpdk.org; Lin, Xueqin <xueqin....@intel.com>; sta...@dpdk.org
> > Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> >
> >
> >
> > > -----Original Message-----
> > > From: Peng, ZhihongX <zhihongx.p...@intel.com>
> > > Sent: Friday, October 15, 2021 4:11 PM
> > > To: david.march...@redhat.com; Burakov, Anatoly
> > > <anatoly.bura...@intel.com>; Ananyev, Konstantin
> > > <konstantin.anan...@intel.com>; step...@networkplumber.org;
> > > Dumitrescu, Cristian <cristian.dumitre...@intel.com>; Mcnamara, John
> > > <john.mcnam...@intel.com>
> > > Cc: dev@dpdk.org; Lin, Xueqin <xueqin....@intel.com>; Peng, ZhihongX
> > > <zhihongx.p...@intel.com>; sta...@dpdk.org
> > > Subject: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> > >
> > > From: Zhihong Peng <zhihongx.p...@intel.com>
> > >
> > > 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: sta...@dpdk.org
> > >
> > > Signed-off-by: Xueqin Lin <xueqin....@intel.com>
> > > Signed-off-by: Zhihong Peng <zhihongx.p...@intel.com>
> > > ---
> > > 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 sta...@dpdk.org? 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