Re: reading uninitialized buffer - Mailing list pgsql-patches

From Andrew Dunstan
Subject Re: reading uninitialized buffer
Date
Msg-id 401D48B4.2040808@dunslane.net
Whole thread Raw
In response to Re: reading uninitialized buffer  (Dennis Bjorklund <db@zigo.dhs.org>)
Responses Re: reading uninitialized buffer
Re: reading uninitialized buffer
List pgsql-patches
OK, then *This* patch does it the way I think is clearest. Most of it is
just reindenting.

cheers

andrew

Dennis Bjorklund wrote:

>On Sun, 1 Feb 2004, Andrew Dunstan wrote:
>
>
>
>>As for the test being outside the "if" statement, it is true that that
>>might waste a few cycles, but it hardly matters.
>>
>>
>
>The cycles are not important. My "fix" wasn't the most optimized either if
>one should count cycles. It was terminating the string twice in some
>cases. That I thought about and came to the conclusion that it was not
>important.  That I didn't rewrite the strncmp() to strcmp() is strange to
>me, the length is obviously not needed. Good thing you looked at it.
>
>
>
>>Personally, I would prefer to replace the if statement with this:
>>
>>    if (c == EOF || c == '\n')
>>    {
>>        *buf = '\0';
>>        return;
>>    }
>>
>>and then it wouldn't be an issue at all, but I know some people don't
>>like early function returns - is there a general postgres style rule
>>
>>
>
>I don't know what the style rules say. I have nothing against early
>returns if used with grace. Early exits for odd cases, before the main
>part of the function, just helps readability if you ask me. On the other
>hand it does not matter since the correct is always to use whatever style
>the rest of the program uses.
>
>
>
Index: src/backend/libpq/hba.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/libpq/hba.c,v
retrieving revision 1.119
diff -c -r1.119 hba.c
*** src/backend/libpq/hba.c    25 Dec 2003 03:44:04 -0000    1.119
--- src/backend/libpq/hba.c    1 Feb 2004 17:17:34 -0000
***************
*** 105,187 ****
      while ((c = getc(fp)) != EOF && (pg_isblank(c) || c == ','))
          ;

!     if (c != EOF && c != '\n')
      {
!         /*
!          * Build a token in buf of next characters up to EOF, EOL,
!          * unquoted comma, or unquoted whitespace.
!          */
!         while (c != EOF && c != '\n' &&
!                (!pg_isblank(c) || in_quote == true))
          {
!             /* skip comments to EOL */
!             if (c == '#' && !in_quote)
!             {
!                 while ((c = getc(fp)) != EOF && c != '\n')
!                     ;
!                 /* If only comment, consume EOL too; return EOL */
!                 if (c != EOF && buf == start_buf)
!                     c = getc(fp);
!                 break;
!             }
!
!             if (buf >= end_buf)
!             {
!                 ereport(LOG,
!                         (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                          errmsg("authentication file token too long, skipping: \"%s\"",
!                                 buf)));
!                 /* Discard remainder of line */
!                 while ((c = getc(fp)) != EOF && c != '\n')
!                     ;
!                 buf[0] = '\0';
!                 break;
!             }
!
!             if (c != '"' || (c == '"' && was_quote))
!                 *buf++ = c;
!
!             /* We pass back the comma so the caller knows there is more */
!             if ((pg_isblank(c) || c == ',') && !in_quote)
!                 break;
!
!             /* Literal double-quote is two double-quotes */
!             if (in_quote && c == '"')
!                 was_quote = !was_quote;
!             else
!                 was_quote = false;
!
!             if (c == '"')
!             {
!                 in_quote = !in_quote;
!                 saw_quote = true;
!             }

!             c = getc(fp);
          }

!         /*
!          * Put back the char right after the token (critical in case it is
!          * EOL, since we need to detect end-of-line at next call).
!          */
!         if (c != EOF)
!             ungetc(c, fp);
      }


      if ( !saw_quote &&
           (
!              strncmp(start_buf,"all",3) == 0  ||
!              strncmp(start_buf,"sameuser",8) == 0  ||
!              strncmp(start_buf,"samegroup",9) == 0
           )
          )
      {
          /* append newline to a magical keyword */
          *buf++ = '\n';
      }

-     *buf = '\0';

  }

--- 105,191 ----
      while ((c = getc(fp)) != EOF && (pg_isblank(c) || c == ','))
          ;

!     if (c == EOF || c == '\n')
      {
!         *buf = '\0';
!         return;
!     }
!
!     /*
!      * Build a token in buf of next characters up to EOF, EOL,
!      * unquoted comma, or unquoted whitespace.
!      */
!     while (c != EOF && c != '\n' &&
!            (!pg_isblank(c) || in_quote == true))
!     {
!         /* skip comments to EOL */
!         if (c == '#' && !in_quote)
          {
!             while ((c = getc(fp)) != EOF && c != '\n')
!                 ;
!             /* If only comment, consume EOL too; return EOL */
!             if (c != EOF && buf == start_buf)
!                 c = getc(fp);
!             break;
!         }

!         if (buf >= end_buf)
!         {
!             ereport(LOG,
!                     (errcode(ERRCODE_CONFIG_FILE_ERROR),
!                      errmsg("authentication file token too long, skipping: \"%s\"",
!                             buf)));
!             /* Discard remainder of line */
!             while ((c = getc(fp)) != EOF && c != '\n')
!                 ;
!             buf[0] = '\0';
!             break;
          }

!         if (c != '"' || (c == '"' && was_quote))
!             *buf++ = c;
!
!         /* We pass back the comma so the caller knows there is more */
!         if ((pg_isblank(c) || c == ',') && !in_quote)
!             break;
!
!         /* Literal double-quote is two double-quotes */
!         if (in_quote && c == '"')
!             was_quote = !was_quote;
!         else
!             was_quote = false;
!
!         if (c == '"')
!         {
!             in_quote = !in_quote;
!             saw_quote = true;
!         }
!
!         c = getc(fp);
      }

+     /*
+      * Put back the char right after the token (critical in case it is
+      * EOL, since we need to detect end-of-line at next call).
+      */
+     if (c != EOF)
+         ungetc(c, fp);
+
+     *buf = '\0';

      if ( !saw_quote &&
           (
!              strcmp(start_buf,"all") == 0  ||
!              strcmp(start_buf,"sameuser") == 0  ||
!              strcmp(start_buf,"samegroup") == 0
           )
          )
      {
          /* append newline to a magical keyword */
          *buf++ = '\n';
+         *buf = '\0';
      }


  }


pgsql-patches by date:

Previous
From: david@fetter.org (David Fetter)
Date:
Subject: Re: Patch for psql startup clarity
Next
From: Peter Eisentraut
Date:
Subject: Re: Patch for psql startup clarity