From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 02 Aug 2016 17:38:23 GMT
Subject: Re: [RFC/PATCH v11 03/13] bisect--helper: `write_terms` shell function in C
Message-ID: <xmqq37mnf4vk.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <0102015640423cd0-ae286008-08b7-4ca4-9b2b-f4226aa80a97-000000@eu-west-1.amazonses.com>

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