Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time limit on -d/-D overly strict #41

Open
kreuvf opened this issue Jun 1, 2024 · 1 comment
Open

Time limit on -d/-D overly strict #41

kreuvf opened this issue Jun 1, 2024 · 1 comment

Comments

@kreuvf
Copy link

kreuvf commented Jun 1, 2024

beep/beep-main.c contains the magic number 300000U four times from lines 268 to 296 restricting, among others, the time for the delay -d and -D switches to 300 s or 5 min.

I used to use beep for simple remind-me-in-x-min tasks (like getting the pizza from the oven 🍕), but now that is not possible anymore (without some wrapper script etc. pp.). Also, calling usage_bail() was unhelpful to me in that case, because the documentation does not state this limit and the program still refuses to work with a perfectly acceptable (according to documentation) invocation.

Maybe I am missing something, but what was the reasoning behind this seemingly arbitrary limit? 🤔

(Also, thank you very much for picking up an unmaintained project 🥺👍)

@ndim
Copy link
Member

ndim commented Jun 7, 2024

TBH, I would solve the remind-me-in-x-min tasks with something like

((sleep 10m && /usr/share/doc/beep/contrib/success-beeps) || /usr/share/doc/beep/contrib/failure-beeps)

Perhaps I should write a nice wrapper script for contrib/?

Anyway, that is probably why I did not give much thought to keeping the max value for the delay times high.

IIRC those limits appeared reasonably long at the time, and limits were introduced to avoid 32bit integer overflows.

I did not spend much thought on making the limits as long as possible. The priority was to be certain that overflows cannot happen.

It appears commit 4d18a95 introduced those constants, changing from 2100000 (equivalent to 2100 seconds or 70 minutes) to 300000 /* 5 minutes */.

The 2100000 value is is from ndim@60a0d26 in response to johnath#13 which was even before I forked beep as https://github.com/spkr-beep/beep.

This limit was introduced when we were using usleep(1000U*some_value). We are now (in https://github.com/spkr-beep/beep/blob/master/beep-main.c#L373) using nanosleep(2) which takes a time_t for the whole seconds and another integer for the sub-second nanoseconds (commit aae362d).

It should be possible to extend the ranges without the risk of integer overflows (guarding them with a number of static_assert(..., ...) statements), and also document the ranges in both the beep --help (https://github.com/spkr-beep/beep/blob/master/beep-usage.txt.in) and the beep(1) man page (https://github.com/spkr-beep/beep/blob/master/beep.1.in).

Let me see what I can do when I am back at a proper computer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants