> > Would you like me to add an interface that'd allow stderr to be
> > redirected in the case of defaultNoticeProcessor()?
> 
> No, the application is supposed to set a notice processor if it
> doesn't want stuff going to stderr.  I don't think that stuff needs
> to be redesigned.

Ehh... I'd say it's kinda broken as a logging interface in its current
encarnation.  Whoever does the autoconf magic should test for
stdarg.h.  Since we're on the verge of a minor version change, might
not be a bad time to adjust this.  There should be no source code
incompatabilities introduced in this change, but things will have to
be recompiled as the function ABI has changed.

> > We can stat a file that is owned by root and not readable to the
> > user.  In such a case, because the user isn't able to read the
> > file, all libpq applications will fail making it effectively
> > possible for the system admin to shut off all applications that
> > use libpq.
> 
> Hardly, since the user can simply remove or rename the file.  It's
> in his home directory, remember?  (If he can't write his home
> directory then whether libpq works is the least of his troubles.)

*shrug* Whatever floats whoever's boat, I'm easy, just playing devils
advocate.  Now that I think about it though, there's no graceful way
to get PasswordFromFile() to tell the application to abort and nor
should it.  I think the warnings and bailing out of PasswordFromFile()
is sufficient.  Comments on the revised patch?  -sc

-- 
Sean Chittenden
Index: fe-connect.c
===================================================================
RCS file: /home/ncvs/pgsql/pgsql-server/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.248
diff -u -r1.248 fe-connect.c
--- fe-connect.c        14 Jun 2003 17:49:53 -0000      1.248
+++ fe-connect.c        20 Jun 2003 23:17:50 -0000
@@ -21,6 +21,9 @@
 #include <errno.h>
 #include <ctype.h>
 #include <time.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
 
 #ifndef HAVE_STRDUP
 #include "strdup.h"
@@ -176,12 +179,11 @@
                           PQExpBuffer errorMessage);
 static char *conninfo_getval(PQconninfoOption *connOptions,
                                const char *keyword);
-static void defaultNoticeProcessor(void *arg, const char *message);
+static void defaultNoticeProcessor(void *arg, const char *fmt, ...);
 static int parseServiceInfo(PQconninfoOption *options,
                                 PQExpBuffer errorMessage);
 static char *pwdfMatchesString(char *buf, char *token);
-static char *PasswordFromFile(char *hostname, char *port, char *dbname,
-                                                         char *username);
+static char *PasswordFromFile(PGconn *conn);
 
 /*
  *             Connecting to a Database
@@ -385,8 +387,7 @@
        {
                if (conn->pgpass)
                        free(conn->pgpass);
-               conn->pgpass = PasswordFromFile(conn->pghost, conn->pgport,
-                                                                               
conn->dbName, conn->pguser);
+               conn->pgpass = PasswordFromFile(conn);
                if (conn->pgpass == NULL)
                        conn->pgpass = strdup(DefaultPassword);
        }
@@ -2778,11 +2779,15 @@
  */
 
 static void
-defaultNoticeProcessor(void *arg, const char *message)
+defaultNoticeProcessor(void *arg, const char *fmt, ...)
 {
+       va_list ap;
        (void) arg;                                     /* not used */
+
        /* Note: we expect the supplied string to end with a newline already. */
-       fprintf(stderr, "%s", message);
+       va_start(ap, fmt);
+       vfprintf(stderr, fmt, ap);
+       va_end(ap);
 }
 
 /*
@@ -2827,27 +2832,36 @@
 
 /* Get a password from the password file. Return value is malloc'd. */
 static char *
-PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
+PasswordFromFile(PGconn *conn)
 {
        FILE       *fp;
        char       *pgpassfile;
        char       *home;
+       char       *hostname;
+       char       *port;
        struct stat stat_buf;
 
 #define LINELEN NAMEDATALEN*5
        char            buf[LINELEN];
 
-       if (dbname == NULL || strlen(dbname) == 0)
+       if (conn == NULL)
                return NULL;
 
-       if (username == NULL || strlen(username) == 0)
+       if (conn->dbName == NULL || strlen(conn->dbName) == 0)
                return NULL;
 
-       if (hostname == NULL)
+       if (conn->pguser == NULL || strlen(conn->pguser) == 0)
+               return NULL;
+
+       if (conn->pghost == NULL)
                hostname = DefaultHost;
+       else
+               hostname = conn->pghost;
 
-       if (port == NULL)
+       if (conn->pgport == NULL)
                port = DEF_PGPORT_STR;
+       else
+               port = conn->pgport;
 
        /* Look for it in the home dir */
        home = getenv("HOME");
@@ -2857,7 +2871,7 @@
        pgpassfile = malloc(strlen(home) + 1 + strlen(PGPASSFILE) + 1);
        if (!pgpassfile)
        {
-               fprintf(stderr, libpq_gettext("out of memory\n"));
+               conn->noticeHook(NULL, libpq_gettext("out of memory\n"));
                return NULL;
        }
 
@@ -2874,7 +2888,7 @@
        /* If password file is insecure, alert the user and ignore it. */
        if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))
        {
-               fprintf(stderr,
+               conn->noticeHook(NULL,
                                libpq_gettext("WARNING: Password file %s has world or 
group read access; permission should be u=rw (0600)\n"),
                                pgpassfile);
                free(pgpassfile);
@@ -2883,9 +2897,14 @@
 #endif
 
        fp = fopen(pgpassfile, "r");
-       free(pgpassfile);
-       if (fp == NULL)
+       if (fp == NULL) {
+               conn->noticeHook(NULL,
+                               libpq_gettext("WARNING: Unable to read password file 
%s: %s\n"),
+                               pgpassfile, strerror(errno));
+               free(pgpassfile);
                return NULL;
+       }
+       free(pgpassfile);
 
        while (!feof(fp))
        {
@@ -2903,10 +2922,10 @@
                if (buf[len - 1] == '\n')
                        buf[len - 1] = 0;
 
-               if ((t = pwdfMatchesString(t, hostname)) == NULL ||
-                       (t = pwdfMatchesString(t, port)) == NULL ||
-                       (t = pwdfMatchesString(t, dbname)) == NULL ||
-                       (t = pwdfMatchesString(t, username)) == NULL)
+               if ((t = pwdfMatchesString(t, conn->pghost)) == NULL ||
+                       (t = pwdfMatchesString(t, conn->pgport)) == NULL ||
+                       (t = pwdfMatchesString(t, conn->dbName)) == NULL ||
+                       (t = pwdfMatchesString(t, conn->pguser)) == NULL)
                        continue;
                ret = strdup(t);
                fclose(fp);
Index: libpq-fe.h
===================================================================
RCS file: /home/ncvs/pgsql/pgsql-server/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.93
diff -u -r1.93 libpq-fe.h
--- libpq-fe.h  8 Jun 2003 17:43:00 -0000       1.93
+++ libpq-fe.h  20 Jun 2003 23:16:59 -0000
@@ -114,7 +114,7 @@
 
 /* PQnoticeProcessor is the function type for the notice-message callback.
  */
-typedef void (*PQnoticeProcessor) (void *arg, const char *message);
+typedef void (*PQnoticeProcessor) (void *arg, const char *message, ...);
 
 /* Print options for PQprint() */
 typedef char pqbool;
---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

Reply via email to