Pranit Bauva <pranit.bauva@xxxxxxxxx> 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. -- 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