]> git.ozlabs.org Git - ppp.git/commitdiff
chat: Clean up usage of clean() function
authorPaul Mackerras <paulus@ozlabs.org>
Mon, 19 Jul 2021 07:41:09 +0000 (17:41 +1000)
committerPaul Mackerras <paulus@ozlabs.org>
Mon, 19 Jul 2021 07:41:09 +0000 (17:41 +1000)
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>
chat/chat.c

index 1639b3e440692f6bbb92da8ab60314d5ad4e15d3..d2d255a1c7c015660adb75b0f9dcc21e6953d964 100644 (file)
@@ -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);