Bug 264162
| Summary: | /bin/sleep: Add optional units (seconds, minutes, hours, days) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Mallory <mallorya> | ||||
| Component: | bin | Assignee: | Stefan Eßer <se> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | CC: | grahamperrin, se | ||||
| Priority: | --- | ||||||
| Version: | CURRENT | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
Comment on attachment 234127 [details]
Allow passing optional units (seconds, minutes, hours, days) to /bin/sleep
Thank you for the contribution!
The patch looks mostly OK. I would have used a variable to catch the number of patterns parsed by sscanf() instead of duplicating that call, but this is a technical detail.
There is one issue, though:
if (sscanf(argv[1], "%lf%c%1s", &d, &unit, buf) != 1)
should be:
if (sscanf(argv[1], "%lf%c%1s", &d, &unit, buf) == 2)
The != 1 test detects the presence of an extra argument following the number, but does not reject the case of more than 1 character following.
Testing for 2 patterns matching makes sure that there was just a floating point number followed by a single character. This catches the case of e.g. "1sx" being passed as the argument, since the extra "s" would be parsed as a 3rd match.
I'll commit the patch with the test of the first sscanf() changed to "== 2".
And I'm considering to add a loop to add up multiple delay values being passed as is done by the coreutils version of sleep, for full functional compatibility with that version, in a follow-up commit.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=34978f7edd15ef5aa9c14a6eecb18ae5d6fd8842 commit 34978f7edd15ef5aa9c14a6eecb18ae5d6fd8842 Author: A. Mallory <mallorya@fastmail.com> AuthorDate: 2022-05-24 07:43:38 +0000 Commit: Stefan Eßer <se@FreeBSD.org> CommitDate: 2022-05-24 07:43:38 +0000 bin/sleep: add support for units other than seconds The coreutils version of this command accepts a unit designation of s, m, h, or d (for seconds, minutes, hours, days) immediately following the number of (fractional) units to delay. The submitted patch has been modified in one detail: the test meant to detect the presence of the unit modified was not specific (!= 1) and would have accepted a non-numeric initial element or extra characters following the union. The committed version accepts only the number immediately followed by one of the defined unit designators and no further characters. PR: 264162 MFC after: 1 week bin/sleep/sleep.1 | 12 ++++++++++-- bin/sleep/sleep.c | 18 +++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) |
Created attachment 234127 [details] Allow passing optional units (seconds, minutes, hours, days) to /bin/sleep `sleep` is more convenient when you can pass `5m` instead of `300`. It reduces the cognitive load on the user if they don't have to make those conversions themselves. I spent a little while making a patch that mostly works (I'm no C programmer). There are bugs - you can pass `1sx` and it will parse that as `1s` and ignore the `x`. If this new feature is acceptable to the FreeBSD project, the implementation would need some cleaning up. I realize this is a feature that's present in GNU sleep. For the record, I read the GNU sleep man page to see which units it supports but I did not read the source of the program.