On 10/03/2026 01:21, Collin Funk wrote:
I don't know much about splice or vmsplice, but this code looks
reasonable.
Regarding the alignment comment, perhaps it is worth mentioning it is
only needed for SPLICE_F_GIFT [1]? The man page says for that flag,
"Data must also be properly page aligned, both in memory and length." I
don't see any other alignment requirements. It is fine to align the
memory even if we don't use SPLICE_F_GIFT, of course.
yes it's only needed for GIFT but should help CPU cache line efficiency
otherwise.
I'll clarify with comments.
One last thing, is there any reason we have to use size_t over idx_t
here? I don't think it makes a huge deal here, just sticking to
conventions. Also, it is what alignalloc expects.
I'll review use of idx_t.
int
main (int argc, char **argv)
{
@@ -117,18 +258,22 @@ main (int argc, char **argv)
while (++operandp < operand_lim);
buf[bufused - 1] = '\n';
- /* If a larger buffer was allocated, fill it by repeating the buffer
- contents. */
size_t copysize = bufused;
- for (size_t copies = bufalloc / copysize; --copies; )
+
+ /* Repeatedly output the buffer until there is a write error; then fail.
+ Do a minimal write first to check output with minimal set up cost.
+ If successful then set up for efficient repetition. */
+ if (full_write (STDOUT_FILENO, buf, copysize) == copysize
+ && splice_write (buf, copysize) == 0)
Maybe I am interpreting this comment incorrectly, but it makes me think
that the only reason write is used is as an optimization. However, isn't
it necessary to be able to check for write errors? That is, it is more
than an optimization.
For example, I don't think vmsplice would diagnose this error:
$ yes > /dev/full
yes: standard output: No space left on device
I haven't tested it, so I could be wrong. But I do not see ENOSPC or
many other errors documented in the man page [2].
That's a gap in the man page.
It should say that write(2) errors can occur.
I tested (did most testing) without the initial write() actually,
and all cases were handled fine by vmsplice().
diff --git a/tests/misc/yes.sh b/tests/misc/yes.sh
I think we should also test that 'yes' works even if it cannot open file
descriptors. For example, this command should succeed:
$ (ulimit -n 4; yes)
It does work with your commit, but I think it is worth testing. Note
that many shells cannot handle file descriptor limits that low, as I
found out when writing an 'ls' test [3]. Using strace to make pipe/pipe2
fail is probably adequate.
ack.
thanks for the review!
Padraig
