Hi Pranit,

On 12/16/2016 08:35 PM, Pranit Bauva wrote:

On Thu, Nov 17, 2016 at 5:17 AM, Stephan Beyer [off-list ref] wrote:

quoted

On 10/14/2016 04:14 PM, Pranit Bauva wrote:

quoted

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d84ba86..c542e8b 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c

@@ -123,13 +123,40 @@ static int bisect_reset(const char *commit)
      return bisect_clean_state();
 }

+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) >= 40) {
+             strbuf_trim(&actual_hex);
+             res = !strcmp(actual_hex.buf, expected_hex);
+     }
+     strbuf_release(&actual_hex);
+     return res;
+}

I am not sure it does what it should.

I would expect the following behavior from this function:
 - file does not exist (or is "broken") => return 0
 - actual_hex != expected_hex => return 0
 - otherwise return 1

If I am not wrong, the code does the following instead:
 - file does not exist (or is "broken") => return 0
 - actual_hex != expected_hex => return 1
 - otherwise => return 0

It seems that I didn't carefully see what the shell code is (or
apparently did a mistake in understanding it ;)). I think the C
version does exactly what the shell version does. Can you confirm it
once again, please?

I check again...

The shell code does the following:
 - file does not exist or is not a regular file => return false
 - actual_hex != expected_hex => return false
 - otherwise => return true

(false means a value != 0, true means 0)

Your code does the following:
 - file cannot be read or is broken => return 0
 - actual_hex != expected_hex => return 0
 - otherwise => return 1

So you are very right, it seems I had a weird thinko (I probably missed
the "!" in front of strcmp or something) :)

Sorry for bothering,
  Stephan