code review (backport from tcl-core);

closes ticket [a858d95f4bfddafb]: adjust word-token pointer after possible realloc
sebres · May 7, 2024 · c2cecce · c2cecce
1 parent 192ea54
commit c2cecce
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 46 deletions.
diff --git a/generic/tclClockFmt.c b/generic/tclClockFmt.c
@@ -2075,14 +2075,13 @@ EstimateTokenCount(
     return ++tokcnt;
 }
 
-#define AllocTokenInChain(tok, chain, tokCnt) \
-    if (++(tok) >= (chain) + (tokCnt)) { \
-	chain = ckrealloc((char *)(chain), \
+#define AllocTokenInChain(tok, chain, tokCnt)				 \
+    if (++(tok) >= (chain) + (tokCnt)) {				 \
+	chain = ckrealloc((char *)(chain),				 \
 	    (tokCnt + CLOCK_MIN_TOK_CHAIN_BLOCK_SIZE) * sizeof(*(tok))); \
-	if ((chain) == NULL) { goto done; }; \
-	(tok) = (chain) + (tokCnt); \
-	(tokCnt) += CLOCK_MIN_TOK_CHAIN_BLOCK_SIZE; \
-    } \
+	(tok) = (chain) + (tokCnt);					 \
+	(tokCnt) += CLOCK_MIN_TOK_CHAIN_BLOCK_SIZE;			 \
+    }									 \
     memset(tok, 0, sizeof(*(tok)));
 
 /*
@@ -2210,37 +2209,47 @@ ClockGetOrParseScanFormat(
 	    }
 	    break;
 	    default:
-	    if ( *p == ' ' || isspace(UCHAR(*p)) ) {
-		tok->map = &ScnSpaceTokenMap;
-		tok->tokWord.start = p++;
-		while (p < e && isspace(UCHAR(*p))) {
-		    p++;
-		}
-		tok->tokWord.end = p;
-		/* increase space count used in format */
-		fss->scnSpaceCount++;
-		/* next token */
-		AllocTokenInChain(tok, scnTok, fss->scnTokC); tokCnt++;
-		continue;
-	    }
-word_tok:
-	    if (1) {
-		ClockScanToken	 *wordTok = tok;
-		if (tok > scnTok && (tok-1)->map == &ScnWordTokenMap) {
-		    wordTok = tok-1;
+		if (isspace(UCHAR(*p))) {
+		    tok->map = &ScnSpaceTokenMap;
+		    tok->tokWord.start = p++;
+		    while (p < e && isspace(UCHAR(*p))) {
+			p++;
+		    }
+		    tok->tokWord.end = p;
+		    /* increase space count used in format */
+		    fss->scnSpaceCount++;
+		    /* next token */
+		    AllocTokenInChain(tok, scnTok, fss->scnTokC); tokCnt++;
+		    continue;
 		}
-		/* new word token */
-		if (wordTok == tok) {
+	      word_tok:
+		{
+		/* try continue with previous word token */
+		ClockScanToken *wordTok = tok - 1;
+
+		if (wordTok < scnTok || wordTok->map != &ScnWordTokenMap) {
+		    /* start with new word token */
+		    wordTok = tok;
 		    wordTok->tokWord.start = p;
 		    wordTok->map = &ScnWordTokenMap;
+		}
+
+		do {
+		    if (isspace(UCHAR(*p))) {
+			fss->scnSpaceCount++;
+		    }
+		    p = Tcl_UtfNext(p);
+		} while (p < e && *p != '%');
+		wordTok->tokWord.end = p;
+
+		if (wordTok == tok) {
 		    AllocTokenInChain(tok, scnTok, fss->scnTokC); tokCnt++;
 		}
-		if (isspace(UCHAR(*p))) {
-		    fss->scnSpaceCount++;
+
+
 		}
-		p = Tcl_UtfNext(p);
-		wordTok->tokWord.end = p;
-	    }
+
+
 	    break;
 	    }
 	}
@@ -2273,9 +2282,8 @@ ClockGetOrParseScanFormat(
 	fss->scnTok = scnTok;
 	fss->scnTokC = tokCnt;
     }
-done:
-    Tcl_MutexUnlock(&ClockFmtMutex);
 
+    Tcl_MutexUnlock(&ClockFmtMutex);
     return fss;
 }
 
@@ -3241,20 +3249,29 @@ ClockGetOrParseFmtFormat(
 	    }
 	    break;
 	    default:
-word_tok:
-	    if (1) {
-		ClockFormatToken *wordTok = tok;
-		if (tok > fmtTok && (tok-1)->map == &FmtWordTokenMap) {
-		    wordTok = tok-1;
-		}
-		if (wordTok == tok) {
+	      word_tok:
+		{
+		/* try continue with previous word token */
+		ClockFormatToken *wordTok = tok - 1;
+
+		if (wordTok < fmtTok || wordTok->map != &FmtWordTokenMap) {
+		    /* start with new word token */
+		    wordTok = tok;
 		    wordTok->tokWord.start = p;
 		    wordTok->map = &FmtWordTokenMap;
-		    AllocTokenInChain(tok, fmtTok, fss->fmtTokC); tokCnt++;
 		}
-		p = Tcl_UtfNext(p);
+		do {
+		    p = Tcl_UtfNext(p);
+		} while (p < e && *p != '%');
 		wordTok->tokWord.end = p;
-	    }
+
+		if (wordTok == tok) {
+		    AllocTokenInChain(tok, fmtTok, fss->fmtTokC); tokCnt++;
+		}
+
+
+
+		}
 	    break;
 	    }
 	}
@@ -3271,9 +3288,8 @@ ClockGetOrParseFmtFormat(
 	fss->fmtTok = fmtTok;
 	fss->fmtTokC = tokCnt;
     }
-done:
-    Tcl_MutexUnlock(&ClockFmtMutex);
 
+    Tcl_MutexUnlock(&ClockFmtMutex);
     return fss;
 }
 

diff --git a/tests/clock.test b/tests/clock.test
@@ -363,6 +363,10 @@ test clock-1.9 "clock arguments: option doubly present" {
     list [catch {clock format 0 -gmt 1 -gmt 0} result] $result
 } {1 {bad option "-gmt": doubly present}}
 
+test clock-1.10 {clock format: text with token (bug [a858d95f4bfddafb])} {
+    clock format 0 -format text(%d) -gmt 1
+} {text(01)}
+
 # BEGIN testcases2
 
 # Test formatting of Gregorian year, month, day, all formats
@@ -18908,6 +18912,10 @@ test clock-6.22.20 {Greedy match (second space wins as date-time separator)} {
     clock format [clock scan "111 2 13120" -format "%y%m%d %H%M%S" -gmt 1] -locale en -gmt 1
 } {Sun Jan 02 13:12:00 GMT 2011}
 
+test clock-6.23 {clock scan: text with token (bug [a858d95f4bfddafb])} {
+    clock scan {text(01)} -format text(%d) -gmt 1 -base 0
+} 0
+
 
 test clock-7.1 {Julian Day} {
     clock scan 0 -format %J -gmt true