Refactor condition handling to use offset instead of pointer

If wmc_else() is called after wmb_expand() it can overwrite pointer to
old value. To avoid this issue refactor condition code to use offset
into parsed string instead.

Reported-by: pippin@gimp.org
Signed-off-by: Amadeusz Sławiński <amade@asmblr.net>

3 files changed, 30 insertions, 27 deletions

diff --git a/src/winmsg.c b/src/winmsg.c
index 79936e7..9fd1151 100644
--- a/src/winmsg.c
+++ b/src/winmsg.c
@@ -465,7 +465,7 @@ winmsg_esc_ex(Cond, int *condrend)
 {
 	if (wmc_is_active(cond)) {
 		bool chg;
-		wmbc->p = wmc_end(cond, wmbc->p, &chg);
+		wmbc->p = wmbc->buf->buf + wmc_end(cond, wmbc_offset(wmbc), &chg);
 
 		if (chg)
 			wmbc->buf->numrend = *condrend;
@@ -474,7 +474,7 @@ winmsg_esc_ex(Cond, int *condrend)
 		return;
 	}
 
-	wmc_init(cond, wmbc->p);
+	wmc_init(cond, wmbc_offset(wmbc));
 	*condrend = wmbc->buf->numrend;
 }
 
@@ -482,7 +482,7 @@ winmsg_esc_ex(CondElse, int *condrend)
 {
 	if (wmc_is_active(cond)) {
 		bool chg;
-		wmbc->p = wmc_else(cond, wmbc->p, &chg);
+		wmbc->p = wmbc->buf->buf + wmc_else(cond, wmbc_offset(wmbc), &chg);
 
 		/* if the true branch was discarded, restore to previous rendition
 		 * state; otherwise, we're keeping it, so update the rendition state */
@@ -664,7 +664,7 @@ char *MakeWinMsgEv(WinMsgBuf *winmsg, char *str, Window *win,
 		}
 	}
 	if (wmc_is_active(cond) && !wmc_is_set(cond))
-		wmbc->p = wmc_end(cond, wmbc->p, NULL) + 1;
+		wmbc->p = wmbc->buf->buf + wmc_end(cond, wmbc_offset(wmbc), NULL) + 1;
 	wmbc_putchar(wmbc, '\0' );
 	wmbc->p--; /* TODO: temporary to work with old code */
 	if (numpad) {
diff --git a/src/winmsgcond.c b/src/winmsgcond.c
index 653f34a..fcebb20 100644
--- a/src/winmsgcond.c
+++ b/src/winmsgcond.c
@@ -30,10 +30,11 @@
 
 /* Initialize new condition and set to false; can be used to re-initialize a
  * condition for re-use */
-void wmc_init(WinMsgCond *cond, char *pos)
+void wmc_init(WinMsgCond *cond, int offset)
 {
 	cond->locked = false;
-	cond->pos = pos;
+	cond->offset = offset;
+	cond->initialized = true;
 	wmc_clear(cond);
 }
 
@@ -58,7 +59,7 @@ void wmc_clear(WinMsgCond *cond)
 /* Determine if condition is active (has been initialized and can be used) */
 bool wmc_is_active(const WinMsgCond *cond)
 {
-	return (cond->pos != NULL);
+	return cond->initialized;
 }
 
 /* Determine if a condition is true; the result is undefined if
@@ -69,7 +70,7 @@ bool wmc_is_set(const WinMsgCond *cond)
 }
 
 /* "else" encounted */
-char *wmc_else(WinMsgCond *cond, char *pos, bool *changed)
+int wmc_else(WinMsgCond *cond, int offset, bool *changed)
 {
 	assert(wmc_is_active(cond));
 
@@ -79,19 +80,19 @@ char *wmc_else(WinMsgCond *cond, char *pos, bool *changed)
 	 * instead of the beginning of the true condition---that is, we're accepting
 	 * the destination string up until this point */
 	if (wmc_is_set(cond)) {
-		wmc_init(cond, pos);  /* track this as a new condition */
+		wmc_init(cond, offset);  /* track this as a new condition */
 		cond->locked = true;  /* "else" shall never succeed at this point */
 
 		/* we want to keep the string we have so far (the truth string) */
 		if (changed)
 			*changed = false;
-		return pos;
+		return offset;
 	}
 
 	/* now that we have reached "else" and are not true, we can never be true;
 	 * discard the truth part of the string */
-	char *prevpos = cond->pos;
-	cond->pos = pos;
+	int prevoffset = cond->offset;
+	cond->offset = offset;
 
 	/* the "else" part must always be true at this point, because the previous
 	 * condition failed */
@@ -100,19 +101,19 @@ char *wmc_else(WinMsgCond *cond, char *pos, bool *changed)
 
 	if (changed)
 		*changed = true;
-	return prevpos;
+	return prevoffset;
 }
 
 /* End condition and determine if string should be reset or kept---if our value
  * is truthful, then accept the string, otherwise reject and reset to the
  * position that we were initialized with */
-char *wmc_end(const WinMsgCond *cond, char *pos, bool *changed)
+int wmc_end(const WinMsgCond *cond, int offset, bool *changed)
 {
 	bool set = wmc_is_set(cond);
 	if (changed)
 		*changed = !set;
 
-	return (set) ? pos : cond->pos;
+	return (set) ? offset : cond->offset;
 }
 
 /* Deactivate a condition, preventing its use; this allows a single allocation
@@ -120,6 +121,7 @@ char *wmc_end(const WinMsgCond *cond, char *pos, bool *changed)
 void wmc_deinit(WinMsgCond *cond)
 {
 	cond->state = false;
-	cond->pos = NULL;
+	cond->offset = 0;
 	cond->locked = true;
+	cond->initialized = false;
 }
diff --git a/src/winmsgcond.h b/src/winmsgcond.h
index 9423b3c..4326cd6 100644
--- a/src/winmsgcond.h
+++ b/src/winmsgcond.h
@@ -28,19 +28,20 @@
 
 /* represents a window message condition (e.g. %?)*/
 typedef struct {
-	char *pos;     /* starting position in dest string */
-	bool  state;   /* conditional truth value */
-	bool  locked;  /* when set, prevents state from changing */
+	int  offset;  /* offset in dest string */
+	bool initialized;
+	bool state;   /* conditional truth value */
+	bool locked;  /* when set, prevents state from changing */
 } WinMsgCond;
 
 /* WinMsgCond is intended to be used as an opaque type */
-void  wmc_init(WinMsgCond *, char *);
-void  wmc_set(WinMsgCond *);
-void  wmc_clear(WinMsgCond *);
-bool  wmc_is_active(const WinMsgCond *);
-bool  wmc_is_set(const WinMsgCond *);
-char *wmc_else(WinMsgCond *, char *, bool *);
-char *wmc_end(const WinMsgCond *, char *, bool *);
-void  wmc_deinit(WinMsgCond *);
+void wmc_init(WinMsgCond *, int);
+void wmc_set(WinMsgCond *);
+void wmc_clear(WinMsgCond *);
+bool wmc_is_active(const WinMsgCond *);
+bool wmc_is_set(const WinMsgCond *);
+int  wmc_else(WinMsgCond *, int, bool *);
+int  wmc_end(const WinMsgCond *, int, bool *);
+void wmc_deinit(WinMsgCond *);
 
 #endif