From: | "Marko Kreen" <markokr(at)gmail(dot)com> |
---|---|
To: | "D'Arcy J(dot)M(dot) Cain" <darcy(at)druid(dot)net> |
Cc: | "Josh Berkus" <josh(at)agliodbs(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: The Axe list |
Date: | 2008-10-12 09:57:58 |
Message-ID: | e51f66da0810120257i212448f5g25f8df9e01a92082@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2008-10-12 12:28:07 | Re: recursive query crash |
Previous Message | Gregory Stark | 2008-10-12 08:53:25 | Re: The Axe list |