fix(cron): ensure valid day values; some other stuff

Makefile

@@ -1,6 +1,6 @@
 # =====CONFIG=====
 SRC_DIR := src
-SOURCES != find '$(SRC_DIR)' -iname '*.v'
+SOURCES != find '$(SRC_DIR)' -\( -iname '*.v' -or -iname '*.h' -or -iname '*.c' -\)
 
 V_PATH ?= v
 V := $(V_PATH) -showcc -gc boehm -W -d use_openssl -skip-unused

src/cron/c/expression.c

@@ -84,11 +84,6 @@ int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) {
 
 		while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) {
 			month_index++;
-
-			// TODO find out if this can happen
-			if (month_index == 2 * ce->month_count) {
-				return 1;
-			}
 		}
 	}
 
@@ -108,7 +103,7 @@ int ce_next_from_now(cron_simple_time *out, cron_expression *ce) {
 	struct tm gm;
 	gmtime_r(&t, &gm);
 
-	struct cron_simple_time ref = {
+	cron_simple_time ref = {
 		.year = gm.tm_year,
 		// tm_mon goes from 0 to 11
 		.month = gm.tm_mon + 1,

src/cron/c/expression.h

@@ -1,3 +1,6 @@
+#ifndef VIETER_CRON
+#define VIETER_CRON
+
 #include <time.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -7,7 +10,8 @@ typedef enum cron_parse_error {
 	cron_parse_ok = 0,
 	cron_parse_invalid_expression = 1,
 	cron_parse_invalid_number = 2,
-	cron_parse_out_of_range = 3
+	cron_parse_out_of_range = 3,
+	cron_parse_too_many_parts = 4
 } cron_parse_error;
 
 typedef struct cron_expression {
@@ -38,3 +42,5 @@ int cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *r
 int cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce);
 
 enum cron_parse_error cron_ce_parse_expression(cron_expression *out, char *s);
+
+#endif

src/cron/c/parse.c

@@ -1,21 +1,28 @@
 #include "expression.h"
 
+const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
+
 // Allowed value ranges for the minute, hour, day and month field
 const uint8_t min[4] = {0, 0, 1, 1};
 const uint8_t max[4] = {59, 23, 31, 12};
 
-// Convert a string a uint8_t value by parsing it using atoi and checking
+const uint8_t min_parts = 2;
+const uint8_t max_parts = 4;
+
+// Convert a string into a uint8_t value by parsing it using atoi and checking
 // whether it's contained within the given range
 #define SAFE_ATOI(v,s,min,max) \
 	int _##v = atoi(s); \
 	if ((_##v) == 0 && strcmp((s), "0") != 0) { \
 		return cron_parse_invalid_number; \
 	} \
-	if (v < (min) || v > (max)) { \
+	if (((_##v) < (min)) || ((_##v) > (max))) { \
 		return cron_parse_out_of_range; \
 	} \
 	v = (uint8_t) (_##v);
 
+#define MAX(x, y) (((x) > (y)) ? (x) : (y))
+
 /**
  * Given a range expression, produce a bit field defining what numbers in the
  * min-max range the expression represents. Bit 0 (starting from the
@@ -32,7 +39,7 @@ const uint8_t max[4] = {59, 23, 31, 12};
  * - a/c
  * - a-b/c
  */
-enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) {
+cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) {
 	// The * expression means "every possible value"
 	if (s[0] == '*') {
 		// A '*' is only valid on its own
@@ -98,7 +105,7 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_
  * min-max range the part represents. A part consists of one or more range
  * expressions, separated by commas.
  */
-enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) {
+cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) {
 	*out = 0;
 
 	char *next;
@@ -175,28 +182,30 @@ enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) {
 	s = strdup(s);
 	char *orig_s = s;
 
-	uint8_t part_count = 0;
-
-	char *next;
 	enum cron_parse_error res = cron_parse_ok;
-	uint64_t bfs[4];
+	uint64_t bfs[max_parts];
+
+	// First we divide the input string into its parts, divided by spaces.
+	// Each part is delimited by a NULL byte.
+	uint8_t part_count = 0;
+	char *parts[max_parts];
+	char *next;
 
 	// Skip leading spaces
-	while (s[0] == ' ') {
+	size_t offset = 0;
-		s++;
+
+	while (s[offset] == ' ') {
+		offset++;
 	}
 
-	while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) {
+	s += offset;
+
+	while (part_count < max_parts && ((next = strchr(s, ' ')) != NULL)) {
 		next[0] = '\0';
-		res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]);
+		parts[part_count] = s;
-
-		if (res != cron_parse_ok) {
-			goto end;
-		}
-
-		size_t offset = 1;
 
 		// Skip multiple spaces
+		offset = 1;
 		while (next[offset] == ' ') {
 			offset++;
 		}
@@ -205,34 +214,87 @@ enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) {
 		part_count++;
 	}
 
-	// Parse final trailing part
+	// The loop exited because we already have 4 parts, yet there's still at
-	if (part_count < 4 && s[0] != '\0') {
+	// least one more part that follows.
-		res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]);
+	if (next != NULL) {
+		res = cron_parse_too_many_parts;
+	} else if (s[0] != '\0') {
+		// There's one more excessive trailing part
+		if (part_count == max_parts) {
+			res = cron_parse_too_many_parts;
+			goto end;
+		}
+
+		parts[part_count] = s;
+		part_count++;
+	}
+
+
+	// We now parse the parts in reverse. This is because the month part
+	// determines the maximum value of the day part.
+	
+	uint64_t bit_field = 0;
+	
+	// Months
+	if (part_count >= 4) {
+		res = ce_parse_part(&bit_field, parts[3], min[3], max[3]);
 
 		if (res != cron_parse_ok) {
 			goto end;
 		}
 
-		part_count++;
+		out->month_count = bf_to_nums(&out->months, bit_field, min[3], max[3]);
+	}
+	// If months aren't provided, they're replaced with a *
+	else {
+		out->month_count = bf_to_nums(&out->months, ~0, min[3], max[3]);
 	}
 
-	// At least two parts need to be provided
+	// Determine what the largest allowed day value is, given the months
-	if (part_count < 2) {
+	uint8_t max_day_value = 0;
-		res = cron_parse_invalid_expression;
+
+	for (uint8_t i = 0; i < out->month_count; i++) {
+		max_day_value = MAX(max_day_value, month_days[out->months[i] - 1]);
+	}
+	
+	// Days
+	if (part_count >= 3) {
+		bit_field = 0;
+
+		res = ce_parse_part(&bit_field, parts[2], min[2], max_day_value);
+
+		if (res != cron_parse_ok) {
+			goto end;
+		}
+
+		out->day_count = bf_to_nums(&out->days, bit_field, min[2], max_day_value);
+	}
+	// If days aren't provided, they're replaced with a *
+	else {
+		out->day_count = bf_to_nums(&out->days, ~0, min[2], max_day_value);
+	}
+
+	// Hours
+	bit_field = 0;
+
+	res = ce_parse_part(&bit_field, parts[1], min[1], max[1]);
+
+	if (res != cron_parse_ok) {
 		goto end;
 	}
 
-	// Ensure there's always 4 parts, as expressions can have between 2 and 4 parts
+	out->hour_count = bf_to_nums(&out->hours, bit_field, min[1], max[1]);
-	while (part_count < 4) {
+
-		// Expression is augmented with '*' expressions
+	// Minutes
-		bfs[part_count] = ~0;
+	bit_field = 0;
-		part_count++;
+
+	res = ce_parse_part(&bit_field, parts[0], min[0], max[0]);
+
+	if (res != cron_parse_ok) {
+		goto end;
 	}
 
-	out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]);
+	out->minute_count = bf_to_nums(&out->minutes, bit_field, min[0], max[0]);
-	out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]);
-	out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]);
-	out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]);
 
 end:
 	// s is cloned

src/cron/expression.v

@@ -2,21 +2,22 @@ module cron
 
 import time
 
+[unsafe]
+pub fn (ce &Expression) free() {
+	C.ce_free(ce)
+}
+
 pub fn parse_expression(exp string) !&Expression {
 	out := C.ce_init()
 	res := C.ce_parse_expression(out, exp.str)
 
 	if res != 0 {
-		return error('yuhh')
+		return error(res.str())
 	}
 
 	return out
 }
 
-pub fn (ce &Expression) free() {
-	C.ce_free(ce)
-}
-
 pub fn (ce &Expression) next(ref time.Time) !time.Time {
 	st := SimpleTime{
 		year: ref.year

src/cron/expression_test.v

@@ -22,7 +22,7 @@ fn test_next_simple() ! {
 	// util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')!
 
 	// Overlap to next day
-	mut exp := '0    3        '
+	mut exp := '0    3         '
 	util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')!
 	util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')!