Re: [RFC/PATCH v11 03/13] bisect--helper: `write_terms` shell function in C

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Junio,

On Tue, Aug 2, 2016 at 11:08 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Pranit Bauva <pranit.bauva@xxxxxxxxx> writes:
>
>> +static int write_terms(const char *bad, const char *good)
>> +{
>> +     FILE *fp;
>> +     int res;
>> +
>> +     if (!strcmp(bad, good))
>> +             return error(_("please use two different terms"));
>> +
>> +     if (check_term_format(bad, "bad") || check_term_format(good, "good"))
>> +             return -1;
>> +
>> +     fp = fopen(git_path_bisect_terms(), "w");
>> +     if (!fp)
>> +             return error_errno(_("could not open the file BISECT_TERMS"));
>> +
>> +     res = fprintf(fp, "%s\n%s\n", bad, good);
>> +     res |= fclose(fp);
>> +     return (res < 0) ? -1 : 0;
>> +}
>
> If fprintf(3) were a function that returns 0 on success and negative
> on error (like fclose(3) is), the pattern to cascade the error
> return with "res |= another_call()" is appropriate, but the made me
> hiccup a bit while reading it.  It is not wrong per-se and it would
> certainly be making it worse if we did something silly like
>
>         res = fprintf(...) < 0 ? -1 : 0;
>         res |= fclose(fp);
>
> so I guess what you have is the most succinct way to do this.

I agree with your point and your suggested code is better!

Regards,
Pranit Bauva
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]