* [dpdk-dev] [PATCH] test/eal: fix incorrect array sizes
@ 2019-07-31 18:56 David Marchand
2019-07-31 19:57 ` Michael Santana Francisco
0 siblings, 1 reply; 5+ messages in thread
From: David Marchand @ 2019-07-31 18:56 UTC (permalink / raw)
To: dev; +Cc: aconole, msantana
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>
---
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] = {
{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] = {
{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] = {
{prgname, prefix, mp_flag, "-r", "error"},
{prgname, prefix, mp_flag, "-r", "0"},
{prgname, prefix, mp_flag, "-r", "-1"},
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] test/eal: fix incorrect array sizes
2019-07-31 18:56 [dpdk-dev] [PATCH] test/eal: fix incorrect array sizes David Marchand
@ 2019-07-31 19:57 ` Michael Santana Francisco
2019-08-01 7:25 ` David Marchand
0 siblings, 1 reply; 5+ messages in thread
From: Michael Santana Francisco @ 2019-07-31 19:57 UTC (permalink / raw)
To: David Marchand; +Cc: dev, Aaron Conole
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
> ---
> 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]?
> {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]?
> {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]?
> {prgname, prefix, mp_flag, "-r", "error"},
> {prgname, prefix, mp_flag, "-r", "0"},
> {prgname, prefix, mp_flag, "-r", "-1"},
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] test/eal: fix incorrect array sizes
2019-07-31 19:57 ` Michael Santana Francisco
@ 2019-08-01 7:25 ` David Marchand
2019-08-02 13:43 ` Michael Santana Francisco
0 siblings, 1 reply; 5+ messages in thread
From: David Marchand @ 2019-08-01 7:25 UTC (permalink / raw)
To: Michael Santana Francisco; +Cc: dev, Aaron Conole
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.
> > {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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] test/eal: fix incorrect array sizes
2019-08-01 7:25 ` David Marchand
@ 2019-08-02 13:43 ` Michael Santana Francisco
2019-08-02 20:49 ` Thomas Monjalon
0 siblings, 1 reply; 5+ messages in thread
From: Michael Santana Francisco @ 2019-08-02 13:43 UTC (permalink / raw)
To: David Marchand; +Cc: dev, Aaron Conole
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] test/eal: fix incorrect array sizes
2019-08-02 13:43 ` Michael Santana Francisco
@ 2019-08-02 20:49 ` Thomas Monjalon
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2019-08-02 20:49 UTC (permalink / raw)
To: David Marchand; +Cc: dev, Michael Santana Francisco, Aaron Conole
> > > > 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>
> Acked-by: Michael Santana <msantana@redhat.com>
Applied, thanks.
About the title: "incorrect" after "fix" is always useless.
I replaced it with "command" to give a bit more context about
what these arrays are for.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-02 20:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-31 18:56 [dpdk-dev] [PATCH] test/eal: fix incorrect array sizes David Marchand
2019-07-31 19:57 ` Michael Santana Francisco
2019-08-01 7:25 ` David Marchand
2019-08-02 13:43 ` Michael Santana Francisco
2019-08-02 20:49 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).