Re: [dpdk-dev] [PATCH] test/eal: fix incorrect array sizes

[Michael Santana Francisco <msantana@redhat.com>]

On Thu, Aug 1, 2019 at 3:25 AM David Marchand <david.marchand@redhat.com> wrote:
>
> On Wed, Jul 31, 2019 at 9:57 PM Michael Santana Francisco
> <msantana@redhat.com> wrote:
> >
> > On Wed, Jul 31, 2019 at 2:56 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > Following removal of -c and -n options, the array should have been
> > > shrunk to avoid launch_proc to access unitialised strings.
> > >
> > > Fixes: b4dbacdb1ad7 ("test/eal: set core mask/list config only in dedicated test")
> > > Fixes: 501fa9a40298 ("test/eal: set memory channel config only in dedicated test")
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > Good patch, but is it just me or are you off by one in some places? see inline
>
> The "funny" thing is that it went unnoticed, because the strdup
> happens after the test binary forked a child to start a dpdk process.
> This child segfaults, but the test code running in the parent just
> checks that starting this child failed (since it passed incorrect eal
> arguments).
>
>
> > > ---
> > >  app/test/test_eal_flags.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
> > > index 827ea88..5b2c0f5 100644
> > > --- a/app/test/test_eal_flags.c
> > > +++ b/app/test/test_eal_flags.c
> > > @@ -244,7 +244,7 @@ test_whitelist_flag(void)
> > >         snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
> > >  #endif
> > >
> > > -       const char *wlinval[][11] = {
> > > +       const char *wlinval[][7] = {
> > Shoudln't this be wlinval[][6]?
>
> I can see 7 elements in this array.
My apologies, your patch was right from the start . I was counting the
wrong thing
Acked-by: Michael Santana <msantana@redhat.com>
>
> > >                 {prgname, prefix, mp_flag,
> > >                                 pci_whitelist, "error", "", ""},
> > >                 {prgname, prefix, mp_flag,
> > > @@ -310,7 +310,7 @@ test_invalid_b_flag(void)
> > >         snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
> > >  #endif
> > >
> > > -       const char *blinval[][9] = {
> > > +       const char *blinval[][5] = {
> > blinval[][6]?
>
> 5 elements.
>
> > >                 {prgname, prefix, mp_flag, "-b", "error"},
> > >                 {prgname, prefix, mp_flag, "-b", "0:0:0"},
> > >                 {prgname, prefix, mp_flag, "-b", "0:error:0.1"},
> > > @@ -414,7 +414,7 @@ test_invalid_r_flag(void)
> > >         snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
> > >  #endif
> > >
> > > -       const char *rinval[][9] = {
> > > +       const char *rinval[][5] = {
> > rinval[][4]?
>
> Idem.
>
> > >                         {prgname, prefix, mp_flag, "-r", "error"},
> > >                         {prgname, prefix, mp_flag, "-r", "0"},
> > >                         {prgname, prefix, mp_flag, "-r", "-1"},
> > > --
> > > 1.8.3.1
> > >
>
>
> --
> David Marchand