public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

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: Sun, 03 Mar 2024 23:29:48 -0800	[thread overview]
Message-ID: <xmqqfrx6sbdf.fsf@gitster.g> (raw)
In-Reply-To: <ZeVxA1KvN18Y85J_@tanuki> (Patrick Steinhardt's message of "Mon, 4 Mar 2024 07:58:11 +0100")

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.

>> 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
>> 
>> 

next prev parent reply	other threads:[~2024-03-04  7:29 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 [this message]
2024-03-04 16:43       ` Junio C Hamano
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=xmqqfrx6sbdf.fsf@gitster.g \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=ps@pks$(echo .)im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox