Re: [dpdk-dev] [PATCH 1/8] examples/fips_validation: enhance getopt_long usage

[David Marchand <david.marchand@redhat.com>]

Hello Ibtisam,

On Wed, Nov 4, 2020 at 11:00 AM Ibtisam Tariq <ibtisam.tariq@emumba.com> wrote:
> > +               case MBUF_DATAROOM_KEYWORD_NUM:
> > +               {
> > +                       uint32_t data_room_size;
>
> Here, I don't think we need a temp storage.
> If the value is invalid, the parsing and then init will fail.
> You can directly pass &env.mbuf_data_room to parser_read_uint32 and
> check its value.
>
>
>
> >
> > -                               env.mbuf_data_room = data_room_size;
> > -                       } else {
> > +                       if (parser_read_uint32(&data_room_size,
> > +                                       optarg) < 0) {
> >                                 cryptodev_fips_validate_usage(prgname);
> >                                 return -EINVAL;
> >                         }
> > +
> > +                       if (data_room_size == 0 ||
> > +                                       data_room_size > UINT16_MAX) {
> > +                               cryptodev_fips_validate_usage(prgname);
> > +                               return -EINVAL;
> > +                       }
> > +
> > +                       env.mbuf_data_room = data_room_size;
> > +
> >                         break;
> > +               }
>
> The type of env.mbuf_data_room is uint16_t and the temp variable type
> is uint32_t. In my opinion, the temp variable size is bigger than
> env.mbuf_data_room to handle overflow value.

All of it could be moved to a read_uint16 parser.
WDYT?


-- 
David Marchand