Re: [PATCH v7 04/10] pkt-line: add packet_flush_gently()
- From
Lars Schneider <larsxschneider@gmail.com>
- Date
- Sep 15, 2016, 16:42 UTC
- Message-ID
- <385016D3-8298-4273-81E7-876CB25B2789@gmail.com>
- In-Reply-To
- <xmqq37l3v2rk.fsf@gitster.mtv.corp.google.com>
Show 38 quoted lines
> On 13 Sep 2016, at 23:44, Junio C Hamano <gitster@pobox.com> wrote:
>
> Lars Schneider <larsxschneider@gmail.com> writes:
>
>>> On 13 Sep 2016, at 00:30, Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> larsxschneider@gmail.com writes:
>>>
>>>> From: Lars Schneider <larsxschneider@gmail.com>
>>>>
>>>> packet_flush() would die in case of a write error even though for some
>>>> callers an error would be acceptable. Add packet_flush_gently() which
>>>> writes a pkt-line flush packet and returns `0` for success and `-1` for
>>>> failure.
>>>> ...
>>>> +int packet_flush_gently(int fd)
>>>> +{
>>>> + packet_trace("0000", 4, 1);
>>>> + if (write_in_full(fd, "0000", 4) == 4)
>>>> + return 0;
>>>> + error("flush packet write failed");
>>>> + return -1;
>>>
>>> It is more idiomatic to do
>>>
>>> return error(...);
>>>
>>> but more importantly, does the caller even want an error message
>>> unconditionally printed here?
>>>
>>> I suspect that it is a strong sign that the caller wants to be in
>>> control of when and what error message is produced; otherwise it
>>> wouldn't be calling the _gently() variant, no?
>>
>> Agreed!
>
> I am also OK with the current form, too. Those who need to enhance
> it to packet_flush_gently(int fd, int quiet) can come later."caller wants to be in control [...] otherwise it wouldn't be calling the _gently() variant" convinced me. I would like to change it like this:
trace_printf_key(&trace_packet, "flush packet write failed"); return -1;
Objections?
Thanks, Lars