From: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: [BUGS] Bug #756: suggestion: file with password instead |
Date: | 2002-09-06 02:32:40 |
Message-ID: | 200209060232.g862Wel10050@candle.pha.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-patches |
OK, changes made. The pasting wasn't my idea but was there as part of
the original source:
sprintf(psqlrc, "%s/.psqlrc-" PG_VERSION, home);
I thought that was the way to do it so I propogated it consistenly.
No unpropoaged. Thanks for the code review.
---------------------------------------------------------------------------
Tom Lane wrote:
> 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
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Attachment | Content-Type | Size |
---|---|---|
unknown_filename | text/plain | 5.1 KB |
unknown_filename | text/plain | 5.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | pgsql-bugs | 2002-09-06 04:17:48 | Bug #758: show_planner_stats does not work for new PREPARE calls |
Previous Message | Joe Conway | 2002-09-06 01:33:14 | Re: Schemas not available for pl/pgsql %TYPE.... |
From | Date | Subject | |
---|---|---|---|
Next Message | pgsql-bugs | 2002-09-06 04:17:48 | Bug #758: show_planner_stats does not work for new PREPARE calls |
Previous Message | Bruce Momjian | 2002-09-06 00:46:03 | Re: [BUGS] Bug #756: suggestion: file with password instead |