> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Monday, September 9, 2019 5:54 PM
> To: Wang, Ying A <ying.a.w...@intel.com>
> Cc: Zhang, Qi Z <qi.z.zh...@intel.com>; Yang, Qiming
> <qiming.y...@intel.com>; dev@dpdk.org; Zhao1, Wei <wei.zh...@intel.com>
> Subject: Re: [PATCH 2/4] net/ice: rework for generic flow enabling
>
> On 09/09, Wang, Ying A wrote:
> >> >+ice_unregister_parser(struct ice_flow_parser *parser,
> >> >+ struct ice_adapter *ad)
> >> >+{
> >> >+ struct ice_pf *pf = &ad->pf;
> >> >+ struct ice_parser_list *list;
> >> >+ struct ice_flow_parser *p_parser;
> >> >+ void *temp;
> >> >+
> >> >+ switch (parser->stage) {
> >> >+ case ICE_FLOW_STAGE_RSS:
> >> >+ list = &pf->rss_parser_list;
> >> >+ break;
> >> >+ case ICE_FLOW_STAGE_PERMISSION:
> >> >+ list = &pf->perm_parser_list;
> >> >+ break;
> >> >+ case ICE_FLOW_STAGE_DISTRIBUTOR:
> >> >+ list = &pf->dist_parser_list;
> >> >+ break;
> >> >+ default:
> >> >+ return;
> >> >+ }
> >>
> >> The switch blocks in above functions are the same, it's better to use
> >> a common function to reduce the duplicated code.
> >
> >The switch blocks in the above two functions have little difference in the
> default behavior, one is return -EINVAL, the other is just return, for
> register/unregister funcs have different return value types. So, Can I just
> keep
> this format?
> >
>
> Duplication is bad and I think it should be easy to deal with the return type
> difference,
>
> struct ice_prase_list *
> ice_get_parser_list(struct ice_flow_parser *parser,
> struct ice_adapter *ad)
> {
> struct ice_parser_list *list = NULL;
> struct ice_pf *pf = &ad->pf;
>
> switch (parser->stage) {
> case ICE_FLOW_STAGE_RSS:
> list = &pf->rss_parser_list;
> break;
> case ICE_FLOW_STAGE_PERMISSION:
> list = &pf->perm_parser_list;
> break;
> case ICE_FLOW_STAGE_DISTRIBUTOR:
> list = &pf->dist_parser_list;
> break;
> default:
> break;
> }
>
> return list;
> }
>
> Then you just need to check its return value, if it's NULL, simply return
> -EINVAL
> on register and directly return on unregister.
OK, thanks for your guidance. I will fix it in v2.
>
> Thanks,
> Xiaolong
