Hi all,

Here a patch for improving the handling of out-of-bound step sizes

- renamed variables "num1, num2, num3" to "low_, high_, step" to improve readability of the method

- Print Message to stderr when an out of bound stepsize is specified

- Reset the stepsize to the default "1" if an out of bound stepsize is specified


> Actually I quess it does not make much sense to pick a step-size whichis bigger than max/2 ... or is there any use-case ? Personally I would restrict it to max/2.

Even though step-sizes bigger than (max-min)/2 do not make much sense, it is possible to use them in some way (see e.g. [1]). So I kept support for them and used "max-min" to restrict step-size.

For testing, I used the attached cron files and did the following:

$ sudo ./crontab ../example-crontab-invalid
Step size of '500' will be ignored, since it is exceeds the maximum possible step size '59' for the specified entry' Step size of '400' will be ignored, since it is exceeds the maximum possible step size '23' for the specified entry' Step size of '31' will be ignored, since it is exceeds the maximum possible step size '30' for the specified entry' Step size of '12' will be ignored, since it is exceeds the maximum possible step size '11' for the specified entry' Step size of '300' will be ignored, since it is exceeds the maximum possible step size '7' for the specified entry'
$ sudo ./crontab ../example-crontab-invalid2
Step size of '11' will be ignored for this entry, since it is exceeds the maximum possible step size '10' for the specifed range of '10-20' Step size of '20' will be ignored for this entry, since it is exceeds the maximum possible step size '10' for the specifed range of '10-20' Step size of '2' will be ignored for this entry, since it is exceeds the maximum possible step size '1' for the specifed range of '4-4' Step size of '3' will be ignored for this entry, since it is exceeds the maximum possible step size '1' for the specifed range of '1-2'

I hope mai attachments are accepted .. if not, see [2] and [3].

Cheers, Alex

[1] https://en.wikipedia.org/wiki/Cron#Non-standard_characters

[2] */500 */400 */31 */12 */300   root    echo hallo

[3] 0-10/10 10-20/11 10-20/20 4-4/2 1-2/3   root    echo hallo


# /etc/cron.d/anacron: crontab entries for the anacron package

SHELL=/bin/sh
PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin

*/500 */400 */31 */12 */300   root      echo hallo
# /etc/cron.d/anacron: crontab entries for the anacron package

SHELL=/bin/sh
PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin

0-10/10 10-20/11 10-20/20 4-4/2 1-2/3   root    echo hallo
From 1396e495bcf2f1de8cd53e2e1a96495f709ccb58 Mon Sep 17 00:00:00 2001
From: Alexander Schwinn <[email protected]>
Date: Tue, 19 Sep 2023 15:24:01 +0200
Subject: [PATCH] Improve step-size out of bound handling - print message to
 stderr - fallback to default stepsize 1

---
 entry.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/entry.c b/entry.c
index 03273a3..c634535 100644
--- a/entry.c
+++ b/entry.c
@@ -362,26 +362,26 @@ get_range(bits, low, high, names, ch, file)
 	 */
 
 	register int	i;
-	auto int	num1, num2, num3;
+	auto int	low_, high_, step;
 
 	Debug(DPARS|DEXT, ("get_range()...entering, exit won't show\n"))
 
 	if (ch == '*') {
 		/* '*' means "first-last" but can still be modified by /step
 		 */
-		num1 = low;
-		num2 = high;
+		low_ = low;
+		high_ = high;
 		ch = get_char(file);
 		if (ch == EOF)
 			return EOF;
 	} else {
-		if (EOF == (ch = get_number(&num1, low, names, ch, file)))
+		if (EOF == (ch = get_number(&low_, low, names, ch, file)))
 			return EOF;
 
 		if (ch != '-') {
 			/* not a range, it's a single number.
 			 */
-			if (EOF == set_element(bits, low, high, num1))
+			if (EOF == set_element(bits, low, high, low_))
 				return EOF;
 			return ch;
 		} else {
@@ -393,7 +393,7 @@ get_range(bits, low, high, names, ch, file)
 
 			/* get the number following the dash
 			 */
-			ch = get_number(&num2, low, names, ch, file);
+			ch = get_number(&high_, low, names, ch, file);
 			if (ch == EOF)
 				return EOF;
 		}
@@ -413,21 +413,42 @@ get_range(bits, low, high, names, ch, file)
 		 * element id, it's a step size.  'low' is
 		 * sent as a 0 since there is no offset either.
 		 */
-		ch = get_number(&num3, 0, PPC_NULL, ch, file);
+		ch = get_number(&step, 0, PPC_NULL, ch, file);
+
+		/* Make sure the step size makes any sense */
+		if (step > 1) {
+			if (high == high_ && low == low_) {
+				if (step > (high_ - low_)) {
+					fprintf(stderr, "Step size of '%i' will be ignored, since it is exceeds the maximum possible step size '%i' for the specified entry'\n", step, high_ - low_);
+					step = 1;
+				}
+			}
+			else { /* a range was specified */
+				if (high_ == low_) {
+					fprintf(stderr, "Step size of '%i' will be ignored for this entry, since it is exceeds the maximum possible step size '1' for the specifed range of '%i-%i'\n", step, low_, high_);
+					step = 1;
+				}
+				else if (step > (high_ - low_)) {
+					fprintf(stderr, "Step size of '%i' will be ignored for this entry, since it is exceeds the maximum possible step size '%i' for the specifed range of '%i-%i'\n", step, high_ - low_, low_, high_);
+					step = 1;
+				}
+			}
+		}
+
 		if (ch == EOF)
 			return EOF;
 	} else {
 		/* no step.  default==1.
 		 */
-		num3 = 1;
+		step = 1;
 	}
 
-	/* range. set all elements from num1 to num2, stepping
-	 * by num3.  (the step is a downward-compatible extension
+	/* range. set all elements from low_ to high_, stepping
+	 * by 'step'.  (the step is a downward-compatible extension
 	 * proposed conceptually by bob@acornrc, syntactically
 	 * designed then implmented by paul vixie).
 	 */
-	for (i = num1;  i <= num2;  i += num3)
+	for (i = low_;  i <= high_;  i += step)
 		if (EOF == set_element(bits, low, high, i))
 			return EOF;
 
-- 
2.39.2

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

Reply via email to