Junio C Hamano <gits...@pobox.com> writes:
> Duy Nguyen <pclo...@gmail.com> writes:
>> On Thu, Jul 24, 2014 at 4:16 AM, Junio C Hamano <gits...@pobox.com> wrote:
>>>> + if (strbuf_read_file(&sb, path.buf, 0) <= 0 ||
>>>> + !skip_prefix(sb.buf, "ref:", &start))
>>>> + goto done;
>>>> while (isspace(*start))
>>>> end = start;
>>>> while (*end && !isspace(*end))
>>> Not new in this round of update, and may not even be an issue, but:
>>> - Earlier, the code returned early on a negative return value from
>>> read-file (i.e., an error), but this round it also does so for
>>> zero. Intended?
>> Yes. But it does not make any difference. strbuf_read_file returns
>> sb.len, if it's empty, the next skip_prefix would fail anyway.
> Yes but changing < 0 to <= 0 is inconsistent with that; I would
> understand if you changed it to <= 4, which would be consistent with
> the reasoning, though.
Just to make sure. I am not saying changing < 0 to <= 4 is a good
idea. I think checking for strbuf_read_file() failure with < 0 and
then checking for malformatted input (or detached head perhaps?)
with !skip_prefix(), i.e. testing two logically separate things with
two separate conditions concatenated together with ||, would be the
most natural way to express it.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html