Page MenuHomeFreeBSD

read builtin: Empty variables on timeout
ClosedPublic

Authored by bdrewery on Sep 7 2021, 7:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 29 2024, 11:17 PM
Unknown Object (File)
Jan 15 2024, 9:00 PM
Unknown Object (File)
Dec 28 2023, 1:17 AM
Unknown Object (File)
Dec 23 2023, 3:47 AM
Unknown Object (File)
Dec 14 2023, 8:35 PM
Unknown Object (File)
Nov 26 2023, 3:02 PM
Unknown Object (File)
Sep 21 2023, 7:30 AM
Unknown Object (File)
Sep 3 2023, 6:05 PM

Details

Summary

This matches how a non-timeout error is handled.

Test Plan

read10.0 passed before, read11.0 failed before. Both pass.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

freebsd_quinteiro.org added inline comments.
bin/sh/miscbltin.c
234–235

What about a status = (128 +..; goto out; here?

346

And an out: label here? I know gotos aren't always preferred. I saw other uses of them in this code, especially in eval.c.

This is more consistent, and probably more useful (while read loops tend not to treat EOF and read errors differently).

The whole error behaviour does not appear to match what most non-ash shells such as bash, ksh93, mksh and yash do (an immediate read error such as caused by <&- or 0>/dev/null leaves the variables unchanged), but POSIX seems to permit both (Consequences of errors in the read page "Default" pointing to XCU 1.4 Utility Description Defaults).

bin/sh/miscbltin.c
346

It is possible, but I'm not sure it is better.

Note that some ++ would have to be rearranged as well.

bin/sh/tests/builtins/read11.0
5

Existing tests use mktemp -d, placing the fifo into a temporary directory, to avoid the discouraged -u option.

8

Hmm, how portable is this? It is probably faster than forking off a child process to open the fifo for writing, which the standard requires and other tests do.

12

Some other tests do stuff like [ "$r" -gt 128 ] && [ "$(kill -l "$r")" = TERM ] to avoid depending on signal numbers.

bin/sh/miscbltin.c
346

Yep, you're right. This version doesn't set the current argument to the empty string, and the other one does. You would have to ap++ before coming here. Never mind.

bdrewery marked 2 inline comments as done.
  • test feedback
  • avoid races in read11.0 test
This revision is now accepted and ready to land.Sep 17 2021, 3:27 PM
This revision was automatically updated to reflect the committed changes.