Re: [PATCH v14 08/27] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C

> Hey Junio,
>
> On Thu, Aug 25, 2016 at 3:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Pranit Bauva <pranit.bauva@gmail.com> writes:
>>
>>> +static int is_expected_rev(const char *expected_hex)
>>> +{
>>> +     struct strbuf actual_hex = STRBUF_INIT;
>>> +     int res = 0;
>>> +     if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) >= 0) {
>>> +             strbuf_trim(&actual_hex);
>>> +             res = !strcmp(actual_hex.buf, expected_hex);
>>
>> If it is known to have 40-hex:
>>
>>  (1) accepting ">= 0" seems way too lenient.  You only expect a
>>      41-byte file (or 42 if somebody would write CRLF, but I do not
>>      think anybody other than yourself is expected to write into
>>      this file, and you do not write CRLF yourself);
>>
>>  (2) strbuf_trim() is overly loose.  You only want to trim the
>>      terimnating LF and it is an error to have other trailing
>>      whitespaces.
>>
>> I think the latter is not a new problem and it is OK to leave it
>> as-is; limiting (1) to >= 40 may still be a good change, though,
>> because it makes the intention of the code clearer.
>
> I can do the first change. Since this wasn't present in the shell
> code, I will also mention it in the commit message.