Hi Pranit,
On 12/16/2016 08:35 PM, Pranit Bauva wrote:
> On Thu, Nov 17, 2016 at 5:17 AM, Stephan Beyer <s-beyer@xxxxxxx> wrote:
>> On 10/14/2016 04:14 PM, Pranit Bauva wrote:
>>> 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