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

RE: [PATCH v1 1/4] builtin/index-pack.c: change xwrite to write_in_full to allow large sizes.

From
rsbecker@nexbridge.com <rsbecker@nexbridge.com>
Date
Feb 27, 2024, 00:12 UTC
Message-ID
<027001da6911$a727b9d0$f5772d70$@nexbridge.com>
In-Reply-To
<xmqqa5nmkcuz.fsf@gitster.g>
On Monday, February 26, 2024 6:47 PM, Junio C Hamano wrote:
Show 14 quoted lines
><rsbecker@nexbridge.com> writes:
>
>>>The code above loops while input_len is non-zero, and correctly
>>>decrements it by the number of bytes written by xwrite() after each
>>>iteration.
>>>
>>>Assuming that xwrite()/write(2) works how I think it does on NonStop,
>>>I'm not sure I understand why this change is necessary.
>>
>> NonStop has a limited SSIZE_MAX. xwrite only handles that much so
>> anything beyond that gets dropped (not in the above code but in other
>> builtins)
>
>xwrite() caps a single write attempt to MAX_IO_SIZE and can return a
short-write, so anything beyound MAX_IO_SIZE will not even be
>sent to the underlying write(2).  There is a heuristic based on the value
of SSIZE_MAX to define MAX_IO_SIZE in <git-compat-util.h>,
>and if the value given by that heuristics is too large for your platform,
you can tweak your own MAX_IO_SIZE (see the comments in
>that header file).
>
>The caller of xwrite() must be prepared to see a write return with value
less than the length it used to call the function, either because
>of this MAX_IO_SIZE cut-off, or because of the underlying
>write(2) returning after a short write.  As long as the caller is prepared,
like Taylor pointed out, I am not sure why you'd need to change
>it.

I understand. I was involved in xwrite() a few years ago. The problem is that users of xwrite() did not account for that and t7704.9 failed as a result. These changes did fix the issue. I am not sure how to proceed based on the above, however. Continue or recode the callers (which is part of what this does)?

Previous: Junio C HamanoNext: rsbecker@nexbridge.com
Message 6 of 18 in “Change xwrite() to write_in_full() in builtins.”
  1. 0/4 Change xwrite() to write_in_full() in builtins.Randall S. Becker, Feb 26, 2024
  2. 1/4 builtin/index-pack.c: change xwrite to write_in_full to allow large sizes.Randall S. Becker, Feb 26, 2024
  3. Taylor BlauFeb 26, 2024
  4. rsbecker@nexbridge.comFeb 26, 2024
  5. Junio C HamanoFeb 26, 2024
  6. rsbecker@nexbridge.comFeb 27, 2024
  7. rsbecker@nexbridge.comFeb 26, 2024
  8. 2/4 builtin/receive-pack.c: change xwrite to write_in_full to allow large sizes.Randall S. Becker, Feb 26, 2024
  9. rsbecker@nexbridge.comFeb 26, 2024
  10. Junio C HamanoFeb 26, 2024
  11. rsbecker@nexbridge.comFeb 27, 2024
  12. 3/4 builtin/repack.c: change xwrite to write_in_full to allow large sizes.Randall S. Becker, Feb 26, 2024
  13. Junio C HamanoFeb 26, 2024
  14. Jeff KingFeb 27, 2024
  15. Jeff KingFeb 27, 2024
  16. 4/4 builtin/unpack-objects.c: change xwrite to write_in_full to allow large sizes.Randall S. Becker, Feb 26, 2024
  17. Junio C HamanoFeb 26, 2024
  18. rsbecker@nexbridge.comFeb 27, 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.