Re: [PATCH 3/5] checkout --to: no auto-detach if the ref is already checked out

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Thu, Jul 24, 2014 at 4:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> +     if (strbuf_read_file(&sb, path.buf, 0) <= 0 ||
>>>> +         !skip_prefix(sb.buf, "ref:", &start))
>>>> +             goto done;
>>>>       while (isspace(*start))
>>>>               start++;
>>>>       end = start;
>>>>       while (*end && !isspace(*end))
>>>>               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.