Re: [BUGS] Bug #756: suggestion: file with password instead of - Mailing list pgsql-patches

From Tom Lane
Subject Re: [BUGS] Bug #756: suggestion: file with password instead of
Date
Msg-id 9506.1031267396@sss.pgh.pa.us
Whole thread Raw
In response to Re: [BUGS] Bug #756: suggestion: file with password instead of  (Bruce Momjian <pgman@candle.pha.pa.us>)
Responses Re: [BUGS] Bug #756: suggestion: file with password instead
Re: [BUGS] Bug #756: suggestion: file with password instead
List pgsql-patches
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> + #define PSQLHISTORY    "/.psql_history"
> ...
> !             char       *psql_history = (char *) malloc(strlen(home) +
> !                                                 strlen(PSQLHISTORY) + 1);

> !                 sprintf(psql_history, "%s" PSQLHISTORY, home);

This seems like a really ugly coding practice.  The sprintf is hard to
read and absolutely dependent on the assumption that PSQLHISTORY
contains no %.  I'd suggest this pattern:

#define PSQLHISTORY    ".psql_history"
> ...
> !             char       *psql_history = (char *) malloc(strlen(home) +
> !                                                 strlen(PSQLHISTORY) + 2);

> !                 sprintf(psql_history, "%s/%s", home, PSQLHISTORY);

as being easier to read and safer.

In PasswordFromFile():

> +     /* Look for it in the home dir */
> +     home = getenv("HOME");
> +     if (home)
> +     {
> +         pgpassfile = malloc(strlen(home) + strlen(PGPASSFILE) + 1);
> +         if (!pgpassfile)
> +         {
> +             fprintf(stderr, gettext("%s: out of memory\n"), pset.progname);
> +             exit(EXIT_FAILURE);
> +         }
> +     }
> +     else
> +         return NULL;

libpq has no business calling exit().  How about just "return NULL" like
all the other failure cases in that routine?

            regards, tom lane

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [BUGS] Bug #756: suggestion: file with password instead of
Next
From: Joe Conway
Date:
Subject: contrib/tablefunc regression test