From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/3] unpack: replace xwrite() loop with write_in_full()
Date: Mon, 04 Mar 2024 08:43:03 -0800 [thread overview]
Message-ID: <xmqqzfveq76w.fsf@gitster.g> (raw)
In-Reply-To: <xmqqfrx6sbdf.fsf@gitster.g> (Junio C. Hamano's message of "Sun, 03 Mar 2024 23:29:48 -0800")
Junio C Hamano <gitster@pobox.com> writes:
> Patrick Steinhardt <ps@pks.im> writes:
>
>>> - 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.
Actually, I changed my mind. As you said, this is flushing the very
end of the data in the input_buffer[] and nobody will fill() the
input_buffer[] after the call to this function happens.
>>> - while (len) {
>>> ...
>>> - len -= ret;
>>> - offset += ret;
>>> - }
>>> + write_in_full(1, buffer + offset, len);
>>
>> Same here.
Ditto. We are about to pass the control back to the caller that
will exit using the "has_errors" we return from here.
>>
>> Patrick
>>
>>> /* All done */
>>> return has_errors;
>>> --
>>> 2.44.0-84-gb387623c12
>>>
>>>
next prev parent reply other threads:[~2024-03-04 16:43 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-02 19:03 [PATCH 0/3] Auditing use of xwrite() Junio C Hamano
2024-03-02 19:03 ` [PATCH 1/3] unpack: replace xwrite() loop with write_in_full() Junio C Hamano
2024-03-04 6:58 ` Patrick Steinhardt
2024-03-04 7:29 ` Junio C Hamano
2024-03-04 16:43 ` Junio C Hamano [this message]
2024-03-02 19:03 ` [PATCH 2/3] sideband: avoid short write(2) Junio C Hamano
2024-03-02 19:03 ` [PATCH 3/3] repack: check error writing to pack-objects subprocess Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqzfveq76w.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.