git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
>> 
>> 
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 4 of 7 in “Auditing use of xwrite()”
  1. 0/3 Auditing use of xwrite()Junio C Hamano, Mar 2, 2024
  2. 1/3 unpack: replace xwrite() loop with write_in_full()Junio C Hamano, Mar 2, 2024
  3. Patrick SteinhardtMar 4, 2024
  4. Junio C HamanoMar 4, 2024
  5. Junio C HamanoMar 4, 2024
  6. 3/3 repack: check error writing to pack-objects subprocessJunio C Hamano, Mar 2, 2024
  7. 2/3 sideband: avoid short write(2)Junio C Hamano, Mar 2, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.