Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>> +static int do_reset(const char *name, int len)
>> +{
>> + [...]
>> + if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
>> + return -1;
>> +
>> + for (i = 0; i < len; i++)
>> + if (isspace(name[i]))
>> + len = i;
>
> What is the purpose of this loop? I could imagine that it's trying to
> strip all whitespace from the end of 'name', however, to do that it
> would iterate backward, not forward. (Or perhaps it's trying to
> truncate at the first space, but then it would need to invert the
> condition or use 'break'.) Am I missing something obvious?
I must be missing the same thing. Given that the callers of
do_reset(), other than the "bug" thing that passes the hard coded
"onto", uses item->arg/item->arg_len which includes everything after
the insn word on the line in the todo list, I do suspect that the
intention is to stop at the first whitespace char to avoid creating
a ref with whitespace in it, i.e. it is a bug that can be fixed with
s/len = i/break/.
The code probably should further check the resulting string with
check_ref_format() to detect strange chars and char sequences that
make the resulting refname invalid. For example, you would not want
to allow a label with two consecutive periods in it.
