RE: [PATCH v2 3/3] builtin/unpack-objects.c: change xwrite to write_in_full avoid truncation.

[<rsbecker@nexbridge.com>]

On Tuesday, February 27, 2024 1:59 PM, Junio C Hamano wrote:
>"Randall S. Becker" <the.n.e.key@gmail.com> writes:
>
>> From: "Randall S. Becker" <rsbecker@nexbridge.com>
>>
>> This change is required because some platforms do not support file
>> writes of arbitrary sizes (e.g, NonStop). xwrite ends up truncating
>> the output to the maximum single I/O size possible for the destination
>> device if the supplied len value exceeds the supported value.
>> Replacing xwrite with write_in_full corrects this problem. Future
>> optimisations could remove the loop in favour of just calling
write_in_full.
>>
>> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
>> ---
>>  builtin/unpack-objects.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index
>> e0a701f2b3..6935c4574e 100644
>> --- a/builtin/unpack-objects.c
>> +++ b/builtin/unpack-objects.c
>> @@ -680,7 +680,7 @@ int cmd_unpack_objects(int argc, const char
>> **argv, const char *prefix UNUSED)
>>
>>  	/* Write the last part of the buffer to stdout */
>>  	while (len) {
>> -		int ret = xwrite(1, buffer + offset, len);
>> +		int ret = write_in_full(1, buffer + offset, len);
>>  		if (ret <= 0)
>>  			break;
>>  		len -= ret;
>
>Why do we need this with a retry loop that is prepared for short
>write(2) specifically like this?
>
>If xwrite() calls underlying write(2) with too large a value, then your
MAX_IO_SIZE is misconfigured, and the fix should go there, not
>here in a loop that expects a working xwrite() that is allowed to return on
short write(2), I would think.

I experimented with using write_in_full vs. keeping xwrite. With xwrite in
this loop, t7704.9 consistently fails as described in the other thread. With
write_in_full, the code works correctly. I assume there are side-effects
that are present. This change is critical to having the code work on
NonStop. Otherwise git seems to be at risk of actually being seriously
broken if unpack does not work correctly. I am happy to have my series
ignored as long as the problem is otherwise corrected.