Pranit Bauva <pranit.bauva@xxxxxxxxx> writes:
> +static int get_next_word(struct strbuf *line, struct strbuf *word)
> +{
> + int i;
> + for (i = 0; line->buf[i] != ' ' && line->buf[i] != '\0'; i++)
> + strbuf_addch(word, line->buf[i]);
> +
> + return 0;
> +}
This looks like a very non-standard way to use a string buffer. The
function does not even have to take line as a whole strbuf, because
the logic does not even look at line->len and relies on line->buf[]
to be NUL terminated, which means the first parameter should instead
be "const char *buf", and the caller would feed then line.buf to the
function.
And I highly suspect that this is a very suboptimal interface, if
the caller then has to compute up to which byte in the line.buf to
splice away to get to the "next word" again.
A better alternative with higher level of abstraction would instead
be to keep the two strbuf parameters as-is, but have this function
responsible for "moving" the first word of "line" strbuf into "word"
strbuf. Then the caller can repeatedly call this function to get
each word in "word" strbuf until "line" becomes empty if it wants to
implement "a word at a time" operation.
If that higher level of abstraction is too limiting for the caller
(and also that would be just as inefficient as the patch under
discussion), another alternative would be to have the caller
maintain "the current byte position in the input" and
do something like:
int pos = 0;
while (pos < line.len) {
strbuf_reset(&word);
get_next_word(line.buf, pos, word);
pos += word.len;
do a thing on "word";
}
to implement "a word at a time" operation. For this to work,
get_next_word() would need to skip the leading blanks itself, of
course.
In any case, I won't comment on the body of the function too deeply;
it will probably become a lot cleaner if you employed the "retval +
goto finish:" pattern for error handling. Use of dequote seems
correct from the cursory look, too.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html