Junio C Hamano <gitster@xxxxxxxxx> writes: > Duy Nguyen <pclouds@xxxxxxxxx> writes: > >> On Thu, Jul 24, 2014 at 4:16 AM, Junio C Hamano <gitster@xxxxxxxxx> 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. -- 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