From: Paul Mackerras <paulus@ozlabs.org>
Date: Mon, 19 Jul 2021 07:41:09 +0000 (+1000)
Subject: chat: Clean up usage of clean() function
X-Git-Tag: ppp-2.5.0~67
X-Git-Url: https://git.ozlabs.org/?a=commitdiff_plain;h=9c7ba0d42dee5e3f84ecb6e4fcdbefc6c1cd965c;p=ppp.git

chat: Clean up usage of clean() function

In a couple of places, we were calling clean(), which does environment
variable substitution among other things, but then using the original
string not the "cleaned" string when logging a message about what
we're doing.

Also, this removes a couple of checks that the "cleaned" string is not
longer than the original string, which date back to the first version
of the code checked into CVS.  Those checks were appropriate before
environment variable substitution was added in commit eaca954c2d4a
("add -E option to use environment variables, from Andreas Arens") and
dynamic reallocation of the result buffer was added in commit
86dd2eec100d ("clean(): Fix buffer overflow.") but are no longer
necessary.

These changes were prompted by github issue #294 and redhat bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1650539

Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
---

diff --git a/chat/chat.c b/chat/chat.c
index 1639b3e..d2d255a 100644
--- a/chat/chat.c
+++ b/chat/chat.c
@@ -1020,14 +1020,13 @@ void chat_send (register char *s)
 	
 	s1 = clean(s, 0);
 	
-	if (strlen(s1) > strlen(s)
-	    || strlen(s1) + 1 > sizeof(fail_buffer))
+	if (strlen(s1) + 1 > sizeof(fail_buffer))
 	    fatal(1, "Illegal or too-long ABORT string ('%v')", s);
 
 	abort_string[n_aborts++] = s1;
 
 	if (verbose)
-	    msgf("abort on (%v)", s);
+	    msgf("abort on (%v)", s1);
 	return;
     }
 
@@ -1041,8 +1040,7 @@ void chat_send (register char *s)
 	
 	s1 = clean(s, 0);
 	
-	if (strlen(s1) > strlen(s)
-	    || strlen(s1) + 1 > sizeof(fail_buffer))
+	if (strlen(s1) + 1 > sizeof(fail_buffer))
 	    fatal(1, "Illegal or too-long CLR_ABORT string ('%v')", s);
 
         old_max = n_aborts;
@@ -1053,7 +1051,7 @@ void chat_send (register char *s)
 		pack++;
 		n_aborts--;
 		if (verbose)
-		    msgf("clear abort on (%v)", s);
+		    msgf("clear abort on (%v)", s1);
 	    }
 	}
         free(s1);
@@ -1070,14 +1068,13 @@ void chat_send (register char *s)
 	    fatal(2, "Too many REPORT strings");
 	
 	s1 = clean(s, 0);
-	if (strlen(s1) > strlen(s)
-	    || strlen(s1) + 1 > sizeof(fail_buffer))
+	if (strlen(s1) + 1 > sizeof(fail_buffer))
 	    fatal(1, "Illegal or too-long REPORT string ('%v')", s);
 	
 	report_string[n_reports++] = s1;
 	
 	if (verbose)
-	    msgf("report (%v)", s);
+	    msgf("report (%v)", s1);
 	return;
     }
 
@@ -1091,8 +1088,7 @@ void chat_send (register char *s)
 	
 	s1 = clean(s, 0);
 	
-	if (strlen(s1) > strlen(s)
-	    || strlen(s1) + 1 > sizeof(fail_buffer))
+	if (strlen(s1) + 1 > sizeof(fail_buffer))
 	    fatal(1, "Illegal or too-long REPORT string ('%v')", s);
 
 	old_max = n_reports;
@@ -1103,7 +1099,7 @@ void chat_send (register char *s)
 		pack++;
 		n_reports--;
 		if (verbose)
-		    msgf("clear report (%v)", s);
+		    msgf("clear report (%v)", s1);
 	    }
 	}
         free(s1);