code review, don't need special handling to adjust julianDay, date in…

…crement by 24:00 (or by leap second, not supported in validation yet); invalid time by 24:01 or 24:00:01

generic/tclClock.c

@@ -3733,8 +3733,8 @@ ClockScanCommit(
 	}
     }
 
-    /* If seconds overflows the day (no validate but not "24:00" or leap-second case), increase days */
-    if (yySecondOfDay >= SECONDS_PER_DAY + ((info->flags & CLF_TIME) && ((yyHour == 24) || (yySeconds == 60)))) {
+    /* If seconds overflows the day (not valide case, or 24:00), increase days */
+    if (yySecondOfDay >= SECONDS_PER_DAY) {
 	yydate.julianDay += (yySecondOfDay / SECONDS_PER_DAY);
 	yySecondOfDay %= SECONDS_PER_DAY;
     }
@@ -3880,19 +3880,28 @@ ClockValidDate(
 
     if (info->flags & CLF_TIME) {
 	/* hour */
-	if (yyHour < 0 || yyHour > ((yyMeridian == MER24) ? 24 : 12)) {
-	    errMsg = "invalid time (hour)";
-	    errCode = "hour";
-	    goto error;
+	if (yyHour < 0 || yyHour > ((yyMeridian == MER24) ? 23 : 12)) {
+	    /* allow 24:00:00 as special case, see [aee9f2b916afd976] */
+	    if (yyMeridian == MER24 && yyHour == 24) {
+		if (yyMinutes != 0 || yySeconds != 0) {
+		    errMsg = "invalid time";
+		    errCode = "time";
+		    goto error;
+		}
+	    } else {
+		errMsg = "invalid time (hour)";
+		errCode = "hour";
+		goto error;
+	    }
 	}
 	/* minutes */
-	if (yyMinutes < 0 || yyMinutes > 59 || (yyMinutes && (yyHour == 24))) {
+	if (yyMinutes < 0 || yyMinutes > 59) {
 	    errMsg = "invalid time (minutes)";
 	    errCode = "minutes";
 	    goto error;
 	}
-	/* oldscan could return secondOfDay (parsedTime) -1 by invalid time (ex.: 25:00:00) */
-	if (yySeconds < 0 || yySeconds > 59 || yySecondOfDay <= -1 || (yySeconds && (yyHour == 24))) {
+	/* oldscan could return secondOfDay -1 by invalid time (see ToSeconds) */
+	if (yySeconds < 0 || yySeconds > 59 || yySecondOfDay <= -1) {
 	    errMsg = "invalid time";
 	    errCode = "seconds";
 	    goto error;

tests/clock.test

@@ -38453,12 +38453,12 @@ test clock-68.1 {Leap second, minute, hour [f2b5f89c0d], regression test (no val
 	}
 	# check with formatted scan:
 	if {[set t [clock scan $d -gmt 1 -format "%Y-%m-%d %H:%M:%S"]] != $i} {
-	    lappend res "fmt-scan \"$d\" == $t, expected $i"
+	    lappend res "frmt-scan \"$d\" == $t, expected $i"
 	}
     }
-    set res; # must be empty
+    join $res \n; # must be empty
 } -result {}
-test clock-68.1a {Leap second, minute, hour [f2b5f89c0d], regression test (validity check)} -body {
+test clock-68.1a {24:00 time [aee9f2b916], regression test} -body {
     set res {}
     foreach {d i} {
 	"2012-06-30 24:00:00" 1341100800
@@ -38469,10 +38469,10 @@ test clock-68.1a {Leap second, minute, hour [f2b5f89c0d], regression test (valid
 	}
 	# check with formatted scan:
 	if {[set t [clock scan $d -gmt 1 -format "%Y-%m-%d %H:%M:%S"]] != $i} {
-	    lappend res "fmt-scan \"$d\" == $t, expected $i"
+	    lappend res "frmt-scan \"$d\" == $t, expected $i"
 	}
     }
-    set res; # must be empty
+    join $res \n; # must be empty
 } -result {}
 test clock-68.2 {Leap second, "24:00", regression test (validity check)} -constraints !valid_off -body {
     # Leap seconds are not supported. "24:00" is supported.
@@ -38493,10 +38493,10 @@ test clock-68.2 {Leap second, "24:00", regression test (validity check)} -constr
 	}
 	# check with formatted scan:
 	if {![catch { clock scan $d -gmt 1 -format "%Y-%m-%d %H:%M:%S" } t] || ![string match *$i* $t]} {
-	    lappend res "fmt-scan \"$d\" == \"$t\", expected \"$i\""
+	    lappend res "frmt-scan \"$d\" == \"$t\", expected \"$i\""
 	}
     }
-    set res; # must be empty
+    join $res \n; # must be empty
 } -result {}
 
 # cleanup