On Sat, Oct 06, 2018 at 08:40:33AM +0800, Tao Qingyun wrote:
> >> - for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
> >> + for (i = 0; i < argc; i++) {
> >> char *target = NULL;
> >> int flags = 0;
> >>
> >> + strbuf_reset(&bname);
> >
> > This is not "trivial" nor "style fix", though.
> >
> > It is not "trivial" because it also changes the behaviour. Instead
> > of resetting the strbuf each time after the loop body runs, the new
> > code resets it before the loop body, even for the 0-th iteration
> > where the strbuf hasn't even been used at all. It is not a "style
> > fix" because we do not have a particular reason to avoid using the
> > comma operator to perform two simple expressions with side effects
> > inside a single expression.
> >
> Thank you and Jeff for your explanation. I think I get the point now.
>
> The third part of `for` statement is normally for a step. I think it's
> improve readability even it does nothing in the first iteration.
>
> And, should I drop this part and resend the patch? I'm a newbie :).
Sorry for the slow reply. For some reason I do not think your message
here made it to the list (but I don't see anything obviously wrong with
it).
Anyway, yes, I think it is worth dropping this hunk and re-sending the
else-if style fix as a separate patch (you may choose to re-send this
hunk as its own patch, too, if you want to argue for its inclusion, but
there is no sense in holding the actual style fix hostage).
-Peff
