Re: [PATCH 1/3] unpack: replace xwrite() loop with write_in_full()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 4, 2024, 07:29 UTC
- Message-ID
- <xmqqfrx6sbdf.fsf@gitster.g>
- In-Reply-To
- <ZeVxA1KvN18Y85J_@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
Show 15 quoted lines
>> - while (input_len) {
>> - err = xwrite(1, input_buffer + input_offset, input_len);
>> - if (err <= 0)
>> - break;
>> - input_len -= err;
>> - input_offset += err;
>> - }
>> + /* Write the last part of the buffer to stdout */
>> + write_in_full(1, input_buffer + input_offset, input_len);
>
> With this change we stop updating `input_len` and `input_offset`, both
> of which are global variables. Assuming that tests pass this must be
> okay right now given that this is the final part of what we are writing.
> But I wonder whether we shouldn't update those regardless just so that
> these remain consistent?It is probably a good hygiene, even though it may not matter at all for the correctness in the current code.
Thanks for your sharp eyes.
Show 27 quoted lines
>> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
>> index e0a701f2b3..f1c85a00ae 100644
>> --- a/builtin/unpack-objects.c
>> +++ b/builtin/unpack-objects.c
>> @@ -679,13 +679,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
>> use(the_hash_algo->rawsz);
>>
>> /* Write the last part of the buffer to stdout */
>> - while (len) {
>> - int ret = xwrite(1, buffer + offset, len);
>> - if (ret <= 0)
>> - break;
>> - len -= ret;
>> - offset += ret;
>> - }
>> + write_in_full(1, buffer + offset, len);
>
> Same here.
>
> Patrick
>
>> /* All done */
>> return has_errors;
>> --
>> 2.44.0-84-gb387623c12
>>
>>