Re: [patch] smtpd: uninitialized variable

[Owain Ainsworth]

On Fri, Mar 20, 2009 at 11:46:20AM +1030, Matthew Haub wrote:
> Hello,
> 
> On Fri, Mar 20, 2009 at 11:21:44AM +1030, Matthew Haub wrote:
> > The variable 'i' is used uninitalized in lka_encode_credentials().
> 
> "should've been a for loop from the beginning" blambert@
> 
> Index: lka.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
> retrieving revision 1.35
> diff -u -r1.35 lka.c
> --- lka.c     11 Mar 2009 11:11:08 -0000      1.35
> +++ lka.c     20 Mar 2009 01:09:15 -0000
> @@ -1216,7 +1216,7 @@
>           sizeof (buffer) - 1)
>               return 0;
>  
> -     while (i++ < len) {
> +     for (i = 1; i <= len; i++) {
>               if (buffer[i] == ':') {
>                       buffer[i] = '\0';
>                       break;
> 
Looking at the code it wants to replace the first instance of a colon
that isn't the first character with a NUL char. If so the following may
be better. Of course, if it should be the whole string, the second check
is unneeded.

-0-
Index: lka.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
retrieving revision 1.35
diff -u -p -r1.35 lka.c
--- lka.c       11 Mar 2009 11:11:08 -0000      1.35
+++ lka.c       20 Mar 2009 02:50:52 -0000
@@ -1204,8 +1204,7 @@ int
 lka_encode_credentials(char *dest, char *src)
 {
        size_t len;
-       char buffer[MAX_LINE_SIZE];
-       size_t i;
+       char buffer[MAX_LINE_SIZE], *p;
 
        len = strlen(src) + 1;
        if (len < 1)
@@ -1216,12 +1215,9 @@ lka_encode_credentials(char *dest, char 
            sizeof (buffer) - 1)
                return 0;
 
-       while (i++ < len) {
-               if (buffer[i] == ':') {
-                       buffer[i] = '\0';
-                       break;
-               }
-       }
+       if ((p = strchr(buffer, ':')) != NULL && p != buffer)
+               *p = '\0';
+
        if (kn_encode_base64(buffer, len, dest, MAX_LINE_SIZE - 1) == -1)
                return 0;
        return 1;