From: Phillip Wood <redacted>

On Monday, February 26, 2024 11:00 AM, Phillip Wood wrote:

To: rsbecker@nexbridge.com; 'Torsten Bögershausen' <redacted>
Cc: git@vger.kernel.org; Patrick Steinhardt <redacted>
Subject: Re: [BUG] 2.44.0 t7704.9 Fails on NonStop ia64

On 26/02/2024 15:32, Phillip Wood wrote:

quoted

Hi Randal

[cc'ing Patrick for the reftable writer]

On 25/02/2024 20:36, rsbecker@nexbridge.com wrote:

quoted

On Sunday, February 25, 2024 2:20 PM, Torsten Bögershausen wrote:

quoted

On Sun, Feb 25, 2024 at 02:08:35PM -0500, rsbecker@nexbridge.com wrote:

quoted

On Sunday, February 25, 2024 1:45 PM, I wrote:

quoted

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().

quoted

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

quoted

quoted

quoted

quoted

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()?

quoted

quoted

quoted

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.

quoted

quoted

quoted

t/helper/test-path-utils.c:                     if (write(1,
buffer,

count)

quoted

quoted

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

quoted

quoted

quoted

trace2/tr2_dst.c:       bytes = write(fd, buf_line->buf,
buf_line->len);

This caller explicitly says it prefers short writes over retrying

I'm getting caught a bit behind the curve. After rebuilding from master, I'm now getting:

+ test 1708960150 -lt 1708970156
+ test_line_count = 3 cruft.before
+ test_line_count = 2 cruft.after
test_line_count: line count for cruft.after != 2

This is looking more like a different problem than xwrite().