On Fri, 15 Nov 2024 09:13:12 +0000
Bruce Richardson <bruce.richard...@intel.com> wrote:

> On Thu, Nov 14, 2024 at 11:25:08AM -0800, Stephen Hemminger wrote:
> > There was useless loop when looking at the DMA address.
> > It looks like it was meant to skip whitespace before
> > calling strtok.
> > 
> > Good time to replace strtok with strtok_r as well.
> > 
> > Link: https://pvs-studio.com/en/blog/posts/cpp/1179/
> > 
> > Fixes: 623dc9364dc6 ("app/dma-perf: introduce DMA performance test")
> > Cc: cheng1.ji...@intel.com
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Stephen Hemminger <step...@networkplumber.org>  
> 
> One comment inline below. With that fixed:
> 
> Acked-by: Bruce Richardson <bruce.richard...@intel.com>
> 
> > ---
> >  app/test-dma-perf/main.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
> > index 18219918cc..dccb0a3541 100644
> > --- a/app/test-dma-perf/main.c
> > +++ b/app/test-dma-perf/main.c
> > @@ -217,27 +217,27 @@ parse_lcore_dma(struct test_configure *test_case, 
> > const char *value)
> >     struct lcore_dma_map_t *lcore_dma_map;
> >     char *input, *addrs;
> >     char *ptrs[2];
> > -   char *start, *end, *substr;
> > +   char *start, *end, *substr, *saveptr;
> >     uint16_t lcore_id;
> >     int ret = 0;
> >  
> >     if (test_case == NULL || value == NULL)
> >             return -1;
> >  
> > -   input = strndup(value, strlen(value) + 1);
> > +   input = strdup(value);
> >     if (input == NULL)
> >             return -1;
> > -   addrs = input;
> >  
> > -   while (*addrs == '\0')
> > -           addrs++;
> > +   addrs = input;
> > +   while (isspace(*addrs))
> > +           ++addrs;
> >     if (*addrs == '\0') {
> >             fprintf(stderr, "No input DMA addresses\n");
> >             ret = -1;
> >             goto out;
> >     }
> >  
> > -   substr = strtok(addrs, ",");
> > +   substr = strtok_r(addrs, ",", &saveptr);  
> 
> Don't need to use strtok here at all. Just use strchr, and then no need for
> a new temporary variable.

The issue is that the test is passing each comma seperated section to 
rte_strsplit
and that needs the first part to be null terminated.
But using strtok_r in only one spot is wrong, will fix.

Reply via email to