n-t-roff / heirloom-doctools

The Heirloom Documentation Tools: troff, nroff, and related utilities
http://n-t-roff.github.io/heirloom/doctools.html
Other
127 stars 23 forks source link

`cat /dev/null > compat.h` -> `> compat.h` #123

Open nabijaczleweli opened 3 months ago

nabijaczleweli commented 3 months ago

unclear what the cat is doing here

n-t-roff commented 3 months ago

Why do you want to change this? Does it not work for you?

nabijaczleweli commented 3 months ago

it does, but it works just as well (but faster) without the cat

n-t-roff commented 3 months ago

Have you checked that this is POSIX compatible?

nabijaczleweli commented 3 months ago

i wouldn't post it if it weren't 💀

n-t-roff commented 2 months ago

I've read the following:

POSIX specifies that if a command contains redirections but no command name, then the redirections are performed in a subshell environment.

which sounds like a shell is spawned from this redirection without command ;)

Interactively normally the form without a command is used but in (at least historic) scripts the form with "cat /dev/null" is found often. In the end it's not worth to change the existing code.

Or most importantly: have you done benchmarks which show a significant difference?

nabijaczleweli commented 2 months ago

The command is simply optional (cf. POSIX, XCU 2.7 Redirection), screenshot from P1003.1-202x/D3, cf. l. 80531: because both cat f > g and > g are the same type of Simple Command, in which the command name is optional: image

In general, a "subshell environment" doesn't mean "must fork", but "may behave as-if it forked", this is exemplified in image this affects some error conditions (for example, this is the difference between : > /ERROR and cat > /ERROR/> /ERROR; : explicitly violates the subshell).

This means that while, yes, it does say that image for "Commands with no Command Name [...] any redirections shall be performed in a subshell environment", it does also say "it is unspecified whether this subshell environment is the same one as that used for a command substitution within the command" because it's very obvious that you do not need to fork to do a redirexion if you aren't execing anything.

You want to validate this? sure:

$ strace /bin/sh -c "> file"
execve("/bin/sh", ["/bin/sh", "-c", "> file"], 0x7ffff9423930 /* 30 vars */) = 0
brk(NULL)                               = 0x562b4d153000
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fdc47830000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=88630, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 88630, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fdc4781a000
close(3)                                = 0
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20t\2\0\0\0\0\0"..., 832) = 832
pread64(3, "\6\0\0\0\4\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"..., 784, 64) = 784
newfstatat(3, "", {st_mode=S_IFREG|0755, st_size=1922136, ...}, AT_EMPTY_PATH) = 0
pread64(3, "\6\0\0\0\4\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"..., 784, 64) = 784
mmap(NULL, 1970000, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7fdc47639000
mmap(0x7fdc4765f000, 1396736, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x26000) = 0x7fdc4765f000
mmap(0x7fdc477b4000, 339968, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17b000) = 0x7fdc477b4000
mmap(0x7fdc47807000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1ce000) = 0x7fdc47807000
mmap(0x7fdc4780d000, 53072, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7fdc4780d000
close(3)                                = 0
mmap(NULL, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fdc47636000
arch_prctl(ARCH_SET_FS, 0x7fdc47636740) = 0
set_tid_address(0x7fdc47636a10)         = 582640
set_robust_list(0x7fdc47636a20, 24)     = 0
rseq(0x7fdc47637060, 0x20, 0, 0x53053053) = 0
mprotect(0x7fdc47807000, 16384, PROT_READ) = 0
mprotect(0x562b4b80b000, 8192, PROT_READ) = 0
mprotect(0x7fdc47862000, 8192, PROT_READ) = 0
prlimit64(0, RLIMIT_STACK, NULL, {rlim_cur=8192*1024, rlim_max=RLIM64_INFINITY}) = 0
munmap(0x7fdc4781a000, 88630)           = 0
getuid()                                = 1000
getgid()                                = 100
getpid()                                = 582640
rt_sigaction(SIGCHLD, {sa_handler=0x562b4b800dc0, sa_mask=~[RTMIN RT_1], sa_flags=SA_RESTORER, sa_restorer=0x7fdc47675050}, NULL, 8) = 0
geteuid()                               = 1000
getrandom("\x0a\xde\x34\xf4\xa1\x2b\x9f\x79", 8, GRND_NONBLOCK) = 8
brk(NULL)                               = 0x562b4d153000
brk(0x562b4d174000)                     = 0x562b4d174000
getppid()                               = 582637
newfstatat(AT_FDCWD, "/home/nabijaczleweli", {st_mode=S_IFDIR|0755, st_size=109, ...}, 0) = 0
newfstatat(AT_FDCWD, ".", {st_mode=S_IFDIR|0755, st_size=109, ...}, 0) = 0
geteuid()                               = 1000
getegid()                               = 100
rt_sigaction(SIGINT, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
rt_sigaction(SIGINT, {sa_handler=0x562b4b800dc0, sa_mask=~[RTMIN RT_1], sa_flags=SA_RESTORER, sa_restorer=0x7fdc47675050}, NULL, 8) = 0
rt_sigaction(SIGQUIT, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
rt_sigaction(SIGQUIT, {sa_handler=SIG_DFL, sa_mask=~[RTMIN RT_1], sa_flags=SA_RESTORER, sa_restorer=0x7fdc47675050}, NULL, 8) = 0
rt_sigaction(SIGTERM, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
rt_sigaction(SIGTERM, {sa_handler=SIG_DFL, sa_mask=~[RTMIN RT_1], sa_flags=SA_RESTORER, sa_restorer=0x7fdc47675050}, NULL, 8) = 0
openat(AT_FDCWD, "file", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
fcntl(1, F_DUPFD, 10)                   = 10
close(1)                                = 0
fcntl(10, F_SETFD, FD_CLOEXEC)          = 0
dup2(3, 1)                              = 1
close(3)                                = 0
dup2(10, 1)                             = 1
close(10)                               = 0
exit_group(0)                           = ?
+++ exited with 0 +++

notice a clone here? yeah me neither.

Compare to the catted version:

$ strace /bin/sh -c "cat /dev/null > file"
execve("/bin/sh", ["/bin/sh", "-c", "cat /dev/null > file"], 0x7ffde068b050 /* 30 vars */) = 0
brk(NULL)                               = 0x564943bca000
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f0ef649f000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=88630, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 88630, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f0ef6489000
close(3)                                = 0
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20t\2\0\0\0\0\0"..., 832) = 832
pread64(3, "\6\0\0\0\4\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"..., 784, 64) = 784
newfstatat(3, "", {st_mode=S_IFREG|0755, st_size=1922136, ...}, AT_EMPTY_PATH) = 0
pread64(3, "\6\0\0\0\4\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"..., 784, 64) = 784
mmap(NULL, 1970000, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f0ef62a8000
mmap(0x7f0ef62ce000, 1396736, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x26000) = 0x7f0ef62ce000
mmap(0x7f0ef6423000, 339968, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17b000) = 0x7f0ef6423000
mmap(0x7f0ef6476000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1ce000) = 0x7f0ef6476000
mmap(0x7f0ef647c000, 53072, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f0ef647c000
close(3)                                = 0
mmap(NULL, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f0ef62a5000
arch_prctl(ARCH_SET_FS, 0x7f0ef62a5740) = 0
set_tid_address(0x7f0ef62a5a10)         = 584555
set_robust_list(0x7f0ef62a5a20, 24)     = 0
rseq(0x7f0ef62a6060, 0x20, 0, 0x53053053) = 0
mprotect(0x7f0ef6476000, 16384, PROT_READ) = 0
mprotect(0x5649421d8000, 8192, PROT_READ) = 0
mprotect(0x7f0ef64d1000, 8192, PROT_READ) = 0
prlimit64(0, RLIMIT_STACK, NULL, {rlim_cur=8192*1024, rlim_max=RLIM64_INFINITY}) = 0
munmap(0x7f0ef6489000, 88630)           = 0
getuid()                                = 1000
getgid()                                = 100
getpid()                                = 584555
rt_sigaction(SIGCHLD, {sa_handler=0x5649421cddc0, sa_mask=~[RTMIN RT_1], sa_flags=SA_RESTORER, sa_restorer=0x7f0ef62e4050}, NULL, 8) = 0
geteuid()                               = 1000
getrandom("\x04\x64\xfa\xb1\xd0\x8e\xc5\x0e", 8, GRND_NONBLOCK) = 8
brk(NULL)                               = 0x564943bca000
brk(0x564943beb000)                     = 0x564943beb000
getppid()                               = 584552
newfstatat(AT_FDCWD, "/home/nabijaczleweli", {st_mode=S_IFDIR|0755, st_size=109, ...}, 0) = 0
newfstatat(AT_FDCWD, ".", {st_mode=S_IFDIR|0755, st_size=109, ...}, 0) = 0
geteuid()                               = 1000
getegid()                               = 100
rt_sigaction(SIGINT, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
rt_sigaction(SIGINT, {sa_handler=0x5649421cddc0, sa_mask=~[RTMIN RT_1], sa_flags=SA_RESTORER, sa_restorer=0x7f0ef62e4050}, NULL, 8) = 0
rt_sigaction(SIGQUIT, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
rt_sigaction(SIGQUIT, {sa_handler=SIG_DFL, sa_mask=~[RTMIN RT_1], sa_flags=SA_RESTORER, sa_restorer=0x7f0ef62e4050}, NULL, 8) = 0
rt_sigaction(SIGTERM, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
rt_sigaction(SIGTERM, {sa_handler=SIG_DFL, sa_mask=~[RTMIN RT_1], sa_flags=SA_RESTORER, sa_restorer=0x7f0ef62e4050}, NULL, 8) = 0
openat(AT_FDCWD, "file", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
fcntl(1, F_DUPFD, 10)                   = 10
close(1)                                = 0
fcntl(10, F_SETFD, FD_CLOEXEC)          = 0
dup2(3, 1)                              = 1
close(3)                                = 0
newfstatat(AT_FDCWD, "/home/nabijaczleweli/.cargo/bin/cat", 0x7ffdb4e91370, 0) = -1 ENOENT (No such file or directory)
newfstatat(AT_FDCWD, "/home/nabijaczleweli/bin/cat", {st_mode=S_IFREG|0755, st_size=56840, ...}, 0) = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
vfork()                                 = 584556
rt_sigprocmask(SIG_SETMASK, [], ~[KILL STOP RTMIN RT_1], 8) = 0
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 584556
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=584556, si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---
rt_sigreturn({mask=[]})                 = 584556
wait4(-1, 0x7ffdb4e912fc, WNOHANG, NULL) = -1 ECHILD (No child processes)
dup2(10, 1)                             = 1
close(10)                               = 0
exit_group(0)                           = ?
+++ exited with 0 +++

notice how it's exactly the same, down to opening file in the main shell, until it vforks and execs cat (there is no reason to do this).

You want a benchmark? sure:

$ hyperfine -N '/bin/sh -c "> file"' '/bin/sh -c "cat /dev/null > file"'
Benchmark 1: /bin/sh -c "> file"
  Time (mean ± σ):       1.2 ms ±   0.2 ms    [User: 0.8 ms, System: 0.2 ms]
  Range (min … max):     0.9 ms …   1.7 ms    1869 runs

Benchmark 2: /bin/sh -c "cat /dev/null > file"
  Time (mean ± σ):       2.4 ms ±   0.2 ms    [User: 1.8 ms, System: 0.5 ms]
  Range (min … max):     1.9 ms …   3.1 ms    1149 runs

Summary
  '/bin/sh -c "> file"' ran
    2.12 ± 0.34 times faster than '/bin/sh -c "cat /dev/null > file"'

this compares executing ["/bin/sh", "-c", "> file"] with ["/bin/sh", "-c", "cat /dev/null > file"], and we can see this is a full millisecond faster (because it doesn't exec and wait for cat for no reason).

This (naturally) includes dash startup time; if we compare

openat(AT_FDCWD, "file", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3 <0.000067>

with

openat(AT_FDCWD, "file", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3 <0.000067>
+
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0 <0.000013>
vfork()                                 = 629750 <0.000318>
rt_sigprocmask(SIG_SETMASK, [], ~[KILL STOP RTMIN RT_1], 8) = 0 <0.000013>
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 629750 <0.000904>
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=629750, si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---
rt_sigreturn({mask=[]})                 = 629750 <0.000016>
wait4(-1, 0x7ffcfcd16dbc, WNOHANG, NULL) = -1 ECHILD (No child processes) <0.000015>

=> (0.000067+0.000013+0.000318+0.000013+0.000904+0.000016+0.000015)/0.000067 = the version where you don't fork and wait is 20x faster.

A significant difference for sure!

Alhadis commented 2 months ago

Or most importantly: have you done benchmarks which show a significant difference?

This is a configure script that's only executed once; performance shouldn't be a concern here, especially if the difference is being measured in milliseconds. @nabijaczleweli has the right idea; using cat(1) to truncate a file is a textbook UUoC, and we should avoid such antipatterns in our code.