From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
Cc: | PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: [BUGS] Bug #756: suggestion: file with password instead of |
Date: | 2002-09-05 23:09:56 |
Message-ID: | 9506.1031267396@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-patches |
Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2002-09-06 00:46:03 | Re: [BUGS] Bug #756: suggestion: file with password instead |
Previous Message | elein | 2002-09-05 22:42:07 | Re: pg-config --includedir-server |
From | Date | Subject | |
---|---|---|---|
Next Message | Joe Conway | 2002-09-05 23:40:57 | contrib/tablefunc regression test |
Previous Message | Bruce Momjian | 2002-09-05 22:06:15 | Re: [BUGS] Bug #756: suggestion: file with password instead of |