Re: The Axe list

On 10/11/08, D'Arcy J.M. Cain <darcy@druid.net> wrote:
> No need.  I have places to put it up.  I would like to make the
>  following changes for the CVS archives before it is removed though.
>  Any objections?
>
>  Index: chkpass.c
>  ===================================================================
>  RCS file: /cvsroot/pgsql/contrib/chkpass/chkpass.c,v
>  retrieving revision 1.20
>  diff -u -p -u -r1.20 chkpass.c
>  --- chkpass.c   25 Mar 2008 22:42:41 -0000  1.20
>  +++ chkpass.c   11 Oct 2008 19:52:52 -0000
>  @@ -70,6 +70,7 @@ chkpass_in(PG_FUNCTION_ARGS)
>     char       *str = PG_GETARG_CSTRING(0);
>     chkpass    *result;
>     char        mysalt[4];
>  +   static bool random_initialized = false;
>     static char salt_chars[] =
>     "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
>
>  @@ -88,10 +89,16 @@ chkpass_in(PG_FUNCTION_ARGS)
>
>     result = (chkpass *) palloc(sizeof(chkpass));
>
>  +   if (!random_initialized)
>  +   {
>  +       srandom((unsigned int) time(NULL));
>  +       random_initialized = true;
>  +   }
>  +

This is bad idea, postgres already does srandom()

>     mysalt[0] = salt_chars[random() & 0x3f];
>     mysalt[1] = salt_chars[random() & 0x3f];
>  -   mysalt[2] = 0;              /* technically the terminator is not
>  necessary
>  -                                * but I like to play safe */
>  +   mysalt[2] = 0;              /* technically the terminator is not
>  +                                * necessary but I like to play safe */
>     strcpy(result->password, crypt(str, mysalt));
>     PG_RETURN_POINTER(result);
>   }

Comment change only?  Ok.

>  @@ -108,9 +115,11 @@ chkpass_out(PG_FUNCTION_ARGS)
>     chkpass    *password = (chkpass *) PG_GETARG_POINTER(0);
>     char       *result;
>
>  -   result = (char *) palloc(16);
>  -   result[0] = ':';
>  -   strcpy(result + 1, password->password);
>  +   if ((result = (char *) palloc(16)) != NULL)
>  +   {
>  +       result[0] = ':';
>  +       strcpy(result + 1, password->password);
>  +   }

AFAIK palloc() cannot return NULL?

>
>     PG_RETURN_CSTRING(result);
>   }
>  @@ -142,6 +151,9 @@ chkpass_eq(PG_FUNCTION_ARGS)
>     text       *a2 = PG_GETARG_TEXT_PP(1);
>     char        str[9];
>
>  +   if (!a1 || !a2)
>  +       PG_RETURN_BOOL(0);
>  +
>     text_to_cstring_buffer(a2, str, sizeof(str));
>     PG_RETURN_BOOL(strcmp(a1->password, crypt(str, a1->password)) == 0);
>   }
>  @@ -154,6 +166,9 @@ chkpass_ne(PG_FUNCTION_ARGS)
>     text       *a2 = PG_GETARG_TEXT_PP(1);
>     char        str[9];
>
>  +   if (!a1 || !a2)
>  +       PG_RETURN_BOOL(0);
>  +
>     text_to_cstring_buffer(a2, str, sizeof(str));
>     PG_RETURN_BOOL(strcmp(a1->password, crypt(str, a1->password)) != 0);
>
>  }

The functions are already defined as STRICT, so unnecessary.
Also returning non-NULL on NULL input seems to go against SQL style.

-- 
marko