Re: [PATCH v2 3/6] t/helper/test-hashmap.c: avoid using `strtok()`

> On Tue, Apr 18, 2023 at 03:18:49PM -0400, Taylor Blau wrote:
>
> > @@ -159,21 +161,34 @@ int cmd__hashmap(int argc, const char **argv)
> >
> >  	/* process commands from stdin */
> >  	while (strbuf_getline(&line, stdin) != EOF) {
> > -		char *cmd, *p1 = NULL, *p2 = NULL;
> > +		char *cmd, *p1, *p2;
> >  		unsigned int hash = 0;
> >  		struct test_entry *entry;
> >
> > +		/*
> > +		 * Because we memdup() the arguments out of the
> > +		 * string_list before inserting them into the hashmap,
> > +		 * it's OK to set its length back to zero to avoid
> > +		 * re-allocating the items array once per line.
> > +		 *
> > +		 * By doing so, we'll instead overwrite the existing
> > +		 * entries and avoid re-allocating.
> > +		 */
> > +		string_list_setlen(&parts, 0);
> >  		/* break line into command and up to two parameters */
> > -		cmd = strtok(line.buf, DELIM);
> > +		string_list_split_in_place_multi(&parts, line.buf, DELIM, 2);
> > +
>
> I'd argue we can drop this comment now. Having string_list_setlen()
> makes it a blessed pattern, and I don't think there's anything special
> about this caller that makes it more or less so. Obviously yes, the
> string list items won't be valid as we enter a new loop iteration. But
> that is always true of split_in_place(), not to mention strtok(),
> because we are overwriting the buffer in each loop.