Re: The Axe list

On 10/11/08, D'Arcy J.M. Cain <darcy(at)druid(dot)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