Hey Junio, On Tue, Aug 2, 2016 at 11:08 PM, Junio C Hamano <gits...@pobox.com> wrote: > Pranit Bauva <pranit.ba...@gmail.com> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html