Re: [PATCH v2 1/2] Change how HTTP response body is returned

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 4, 2019, 20:13 UTC
Message-ID: <xmqqsgy8b2r7.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190104101149.GA26185@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 6 quoted lines

> The most robust thing would perhaps be:
>
>   fflush(dest->file);
>   ftruncate(fileno(dest->file), 0);
>
> which leaves the handle intact.

An added benefit of that approach is that there is no need for the filename field in the dest structure.

Having a separate filename field could be a positive flexibility (it could be used to open a file, store its FILE* in dest->file, while storing the name of another file in dest->filename), but also a negative flexibility (dest->file possibly pointing to a file different from dest->filename is a source of confusion). As I do not think any current caller that wants such a flexibility, or callers in any immediate future, it probably is an overall plus if we do not have to add the dest->filename field.

Show 5 quoted lines

> (I agree with the rest of your review, especially that it would be
> easier to read if this were split into separate refactor and
> change-behavior steps).
>
> -Peff