Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64

[Phillip Wood <phillip.wood123@gmail.com>]

On 26/02/2024 15:32, Phillip Wood wrote:
> Hi Randal
> 
> [cc'ing Patrick for the reftable writer]
> 
> On 25/02/2024 20:36, rsbecker@nexbridge.com wrote:
>> On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:
>>> On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:
>>>> On Sunday, February 25, 2024 1:45 PM, I wrote:
>>>>> To: git@vger.kernel.org
>>> But I think that this should be used:
>>> write_in_full()
>>
>> My mailer autocorrected, yes, xwrite. write_in_full() would be safe,
>> although a bit redundant since xwrite() does similar things and is 
>> used by
>> write_in_full().
> 
> Note that unlike write_in_full(), xwrite() does not guarantee to write 
> the whole buffer passed to it. In general unless a caller is writing a 
> single byte or writing less than PIPE_BUF bytes to a pipe it should use 
> write_in_full().
> 
>> The question is which call is bad? The cruft stuff is
>> relatively new and I don't know the code.

I should have been clearer that I do not think any of these calls are 
the likely problem for the cruft pack code. I do think the reftable 
writers are worth looking at though for git in general.

For the cruft pack problem you might want to look for suspect xwrite() 
calls where the caller does not handle a short write correctly for 
example under builtin/ we have

builtin/index-pack.c:                   err = xwrite(1, input_buffer + 
input_offset, input_len);
builtin/receive-pack.c:         xwrite(2, msg, sz);
builtin/repack.c:       xwrite(cmd->in, oid_to_hex(oid), 
the_hash_algo->hexsz);
builtin/repack.c:       xwrite(cmd->in, "\n", 1);
builtin/unpack-objects.c:               int ret = xwrite(1, buffer + 
offset, len);

Best Wishes

Phillip

>>>> reftable/writer.c:              int n = w->write(w->write_arg, zeroed,
>>>> w->pending_padding);
>>>> reftable/writer.c:      n = w->write(w->write_arg, data, len);
> 
> Neither of these appear to check for short writes and 
> reftable_fd_write() is a thin wrapper around write(). Maybe 
> reftable_fd_write() should be using write_in_full()?
> 
>>>> run-command.c:                  len = write(io->fd, io->u.out.buf,
> 
> This call to write() looks correct as it is in the io pump loop.
> 
>>>> t/helper/test-path-utils.c:                     if (write(1, buffer,
>> count)
>>>> < 0) >>> t/helper/test-windows-named-pipe.c:             write(1, 
>>>> buf, nbr);
>>>> t/helper/test-windows-named-pipe.c:             write(1, buf, nbr);
> 
> In principle these all look like they are prone to short writes.
> 
>>>> trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf, 
>>>> buf_line->len);
> 
> This caller explicitly says it prefers short writes over retrying
> 
> Best Wishes
> 
> Phillip