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.