Clean up messy API for src/port/thread.c.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 11 Jan 2022 18:46:12 +0000 (13:46 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 11 Jan 2022 18:46:20 +0000 (13:46 -0500)
The point of this patch is to reduce inclusion spam by not needing
to #include <netdb.h> or <pwd.h> in port.h (which is read by every
compile in our tree).  To do that, we must remove port.h's
declarations of pqGetpwuid and pqGethostbyname.

pqGethostbyname is only used, and is only ever likely to be used,
in src/port/getaddrinfo.c --- which isn't even built on most
platforms, making pqGethostbyname dead code for most people.
Hence, deal with that by just moving it into getaddrinfo.c.

To clean up pqGetpwuid, invent a couple of simple wrapper
functions with less-messy APIs.  This allows removing some
duplicate error-handling code, too.

In passing, remove thread.c from the MSVC build, since it
contains nothing we use on Windows.

Noted while working on 376ce3e40.

Discussion: https://postgr.es/m/1634252654444.90107@mit.edu

src/backend/libpq/auth.c
src/bin/psql/common.c
src/include/port.h
src/interfaces/libpq/fe-auth.c
src/interfaces/libpq/fe-auth.h
src/interfaces/libpq/fe-connect.c
src/port/Makefile
src/port/getaddrinfo.c
src/port/path.c
src/port/thread.c
src/tools/msvc/Mkvcbuild.pm

index 9a266beb5dd20afa7daa55ce404bbdf73dada6fa..efc53f31353608431e3e73c86bf66059d89cab64 100644 (file)
@@ -18,6 +18,7 @@
 #include <sys/param.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
+#include <pwd.h>
 #include <unistd.h>
 #ifdef HAVE_SYS_SELECT_H
 #include <sys/select.h>
index f210ccbde850efe374be92d9ddf20de610a55571..3503605a7d1526db5663888ee40bed84d50fce2d 100644 (file)
@@ -10,6 +10,7 @@
 #include <ctype.h>
 #include <limits.h>
 #include <math.h>
+#include <pwd.h>
 #include <signal.h>
 #ifndef WIN32
 #include <unistd.h>                /* for write() */
index 56e3721f6a46af6c55fc8c1c991c3fa6bdbd526f..3d103a2b31bce4a81380941d04d779c165d291fb 100644 (file)
@@ -14,8 +14,6 @@
 #define PG_PORT_H
 
 #include <ctype.h>
-#include <netdb.h>
-#include <pwd.h>
 
 /*
  * Windows has enough specialized port stuff that we push most of it off
@@ -484,18 +482,12 @@ extern char *dlerror(void);
 #define RTLD_GLOBAL 0
 #endif
 
-/* thread.h */
+/* thread.c */
 #ifndef WIN32
-extern int pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer,
-                      size_t buflen, struct passwd **result);
+extern bool pg_get_user_name(uid_t user_id, char *buffer, size_t buflen);
+extern bool pg_get_user_home_dir(uid_t user_id, char *buffer, size_t buflen);
 #endif
 
-extern int pqGethostbyname(const char *name,
-                           struct hostent *resultbuf,
-                           char *buffer, size_t buflen,
-                           struct hostent **result,
-                           int *herrno);
-
 extern void pg_qsort(void *base, size_t nel, size_t elsize,
                     int (*cmp) (const void *, const void *));
 extern int pg_qsort_strcmp(const void *a, const void *b);
index 82fc7cdb9867c1cdc44fedd6eaf03f0b00f1fe50..2edc3f48e2e1331922a3d0fd506ef0ade0c7d5da 100644 (file)
@@ -35,7 +35,6 @@
 #ifndef  MAXHOSTNAMELEN
 #include <netdb.h>             /* for MAXHOSTNAMELEN on some */
 #endif
-#include <pwd.h>
 #endif
 
 #include "common/md5.h"
@@ -1099,14 +1098,17 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
 
 
 /*
- * pg_fe_getauthname
+ * pg_fe_getusername
  *
- * Returns a pointer to malloc'd space containing whatever name the user
- * has authenticated to the system.  If there is an error, return NULL,
- * and append a suitable error message to *errorMessage if that's not NULL.
+ * Returns a pointer to malloc'd space containing the name of the
+ * specified user_id.  If there is an error, return NULL, and append
+ * a suitable error message to *errorMessage if that's not NULL.
+ *
+ * Caution: on Windows, the user_id argument is ignored, and we always
+ * fetch the current user's name.
  */
 char *
-pg_fe_getauthname(PQExpBuffer errorMessage)
+pg_fe_getusername(uid_t user_id, PQExpBuffer errorMessage)
 {
    char       *result = NULL;
    const char *name = NULL;
@@ -1116,17 +1118,13 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
    char        username[256 + 1];
    DWORD       namesize = sizeof(username);
 #else
-   uid_t       user_id = geteuid();
    char        pwdbuf[BUFSIZ];
-   struct passwd pwdstr;
-   struct passwd *pw = NULL;
-   int         pwerr;
 #endif
 
    /*
     * Some users are using configure --enable-thread-safety-force, so we
     * might as well do the locking within our library to protect
-    * pqGetpwuid(). In fact, application developers can use getpwuid() in
+    * getpwuid(). In fact, application developers can use getpwuid() in
     * their application if they use the locking call we provide, or install
     * their own locking function using PQregisterThreadLock().
     */
@@ -1140,21 +1138,10 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
                          libpq_gettext("user name lookup failure: error code %lu\n"),
                          GetLastError());
 #else
-   pwerr = pqGetpwuid(user_id, &pwdstr, pwdbuf, sizeof(pwdbuf), &pw);
-   if (pw != NULL)
-       name = pw->pw_name;
+   if (pg_get_user_name(user_id, pwdbuf, sizeof(pwdbuf)))
+       name = pwdbuf;
    else if (errorMessage)
-   {
-       if (pwerr != 0)
-           appendPQExpBuffer(errorMessage,
-                             libpq_gettext("could not look up local user ID %d: %s\n"),
-                             (int) user_id,
-                             strerror_r(pwerr, pwdbuf, sizeof(pwdbuf)));
-       else
-           appendPQExpBuffer(errorMessage,
-                             libpq_gettext("local user with ID %d does not exist\n"),
-                             (int) user_id);
-   }
+       appendPQExpBuffer(errorMessage, "%s\n", pwdbuf);
 #endif
 
    if (name)
@@ -1170,6 +1157,23 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
    return result;
 }
 
+/*
+ * pg_fe_getauthname
+ *
+ * Returns a pointer to malloc'd space containing whatever name the user
+ * has authenticated to the system.  If there is an error, return NULL,
+ * and append a suitable error message to *errorMessage if that's not NULL.
+ */
+char *
+pg_fe_getauthname(PQExpBuffer errorMessage)
+{
+#ifdef WIN32
+   return pg_fe_getusername(0, errorMessage);
+#else
+   return pg_fe_getusername(geteuid(), errorMessage);
+#endif
+}
+
 
 /*
  * PQencryptPassword -- exported routine to encrypt a password with MD5
index 16d5e1da0f0bbd937414ab67839c00e0090fa48a..f22b3fe6488f71e3970b46436e2497ee377e2b9e 100644 (file)
@@ -20,6 +20,7 @@
 
 /* Prototypes for functions in fe-auth.c */
 extern int pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn);
+extern char *pg_fe_getusername(uid_t user_id, PQExpBuffer errorMessage);
 extern char *pg_fe_getauthname(PQExpBuffer errorMessage);
 
 /* Mechanisms in fe-auth-scram.c */
index a12e0180fdbb3068c3a7488e892871e2290cbfc7..5fc16be849fb272b0574fa36cb2e7c7d2418c1a6 100644 (file)
@@ -2813,10 +2813,7 @@ keep_going:                      /* We will come back to here until there is
                    IS_AF_UNIX(conn->raddr.addr.ss_family))
                {
 #ifndef WIN32
-                   char        pwdbuf[BUFSIZ];
-                   struct passwd pass_buf;
-                   struct passwd *pass;
-                   int         passerr;
+                   char       *remote_username;
 #endif
                    uid_t       uid;
                    gid_t       gid;
@@ -2839,28 +2836,20 @@ keep_going:                     /* We will come back to here until there is
                    }
 
 #ifndef WIN32
-                   passerr = pqGetpwuid(uid, &pass_buf, pwdbuf, sizeof(pwdbuf), &pass);
-                   if (pass == NULL)
-                   {
-                       if (passerr != 0)
-                           appendPQExpBuffer(&conn->errorMessage,
-                                             libpq_gettext("could not look up local user ID %d: %s\n"),
-                                             (int) uid,
-                                             strerror_r(passerr, sebuf, sizeof(sebuf)));
-                       else
-                           appendPQExpBuffer(&conn->errorMessage,
-                                             libpq_gettext("local user with ID %d does not exist\n"),
-                                             (int) uid);
-                       goto error_return;
-                   }
+                   remote_username = pg_fe_getusername(uid,
+                                                       &conn->errorMessage);
+                   if (remote_username == NULL)
+                       goto error_return;  /* message already logged */
 
-                   if (strcmp(pass->pw_name, conn->requirepeer) != 0)
+                   if (strcmp(remote_username, conn->requirepeer) != 0)
                    {
                        appendPQExpBuffer(&conn->errorMessage,
                                          libpq_gettext("requirepeer specifies \"%s\", but actual peer user name is \"%s\"\n"),
-                                         conn->requirepeer, pass->pw_name);
+                                         conn->requirepeer, remote_username);
+                       free(remote_username);
                        goto error_return;
                    }
+                   free(remote_username);
 #else                          /* WIN32 */
                    /* should have failed with ENOSYS above */
                    Assert(false);
@@ -7271,16 +7260,7 @@ pqGetHomeDirectory(char *buf, int bufsize)
 
    home = getenv("HOME");
    if (home == NULL || home[0] == '\0')
-   {
-       char        pwdbuf[BUFSIZ];
-       struct passwd pwdstr;
-       struct passwd *pwd = NULL;
-
-       (void) pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd);
-       if (pwd == NULL)
-           return false;
-       home = pwd->pw_dir;
-   }
+       return pg_get_user_home_dir(geteuid(), buf, bufsize);
    strlcpy(buf, home, bufsize);
    return true;
 #else
index b3754d8940a8959900f9d4a07769779d101d329b..bfe1feb0d425cee88991c6322c3f94f79a2f8358 100644 (file)
@@ -84,6 +84,10 @@ libpgport.a: $(OBJS)
    rm -f $@
    $(AR) $(AROPT) $@ $^
 
+# getaddrinfo.o and getaddrinfo_shlib.o need PTHREAD_CFLAGS (but getaddrinfo_srv.o does not)
+getaddrinfo.o: CFLAGS+=$(PTHREAD_CFLAGS)
+getaddrinfo_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS)
+
 # thread.o and thread_shlib.o need PTHREAD_CFLAGS (but thread_srv.o does not)
 thread.o: CFLAGS+=$(PTHREAD_CFLAGS)
 thread_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS)
index b0ca4c778eda23a22daaeadb848e7af63896e24a..3284c6eb52ab1ada00759888fb00d25feef13dcc 100644 (file)
 #include "port/pg_bswap.h"
 
 
+#ifdef FRONTEND
+static int pqGethostbyname(const char *name,
+                           struct hostent *resultbuf,
+                           char *buffer, size_t buflen,
+                           struct hostent **result,
+                           int *herrno);
+#endif
+
 #ifdef WIN32
 /*
  * The native routines may or may not exist on the Windows platform we are on,
@@ -394,3 +402,39 @@ getnameinfo(const struct sockaddr *sa, int salen,
 
    return 0;
 }
+
+/*
+ * Wrapper around gethostbyname() or gethostbyname_r() to mimic
+ * POSIX gethostbyname_r() behaviour, if it is not available or required.
+ */
+#ifdef FRONTEND
+static int
+pqGethostbyname(const char *name,
+               struct hostent *resultbuf,
+               char *buffer, size_t buflen,
+               struct hostent **result,
+               int *herrno)
+{
+#if defined(ENABLE_THREAD_SAFETY) && defined(HAVE_GETHOSTBYNAME_R)
+
+   /*
+    * broken (well early POSIX draft) gethostbyname_r() which returns 'struct
+    * hostent *'
+    */
+   *result = gethostbyname_r(name, resultbuf, buffer, buflen, herrno);
+   return (*result == NULL) ? -1 : 0;
+#else
+
+   /* no gethostbyname_r(), just use gethostbyname() */
+   *result = gethostbyname(name);
+
+   if (*result != NULL)
+       *herrno = h_errno;
+
+   if (*result != NULL)
+       return 0;
+   else
+       return -1;
+#endif
+}
+#endif                         /* FRONTEND */
index 5ac26f4bcf77fec4da0ff431e72fa72efef3c608..69bb8fe40b77c6c70cc451f20ea16f7e0c378029 100644 (file)
@@ -815,16 +815,7 @@ get_home_path(char *ret_path)
 
    home = getenv("HOME");
    if (home == NULL || home[0] == '\0')
-   {
-       char        pwdbuf[BUFSIZ];
-       struct passwd pwdstr;
-       struct passwd *pwd = NULL;
-
-       (void) pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd);
-       if (pwd == NULL)
-           return false;
-       home = pwd->pw_dir;
-   }
+       return pg_get_user_home_dir(geteuid(), ret_path, MAXPGPATH);
    strlcpy(ret_path, home, MAXPGPATH);
    return true;
 #else
index c1040d4e240672e3e7b5e527d67d13dbb7750ced..23c3fbdf864146edc15023bed934da347892efe7 100644 (file)
  *     use *_r function names if they exit
  *         (*_THREADSAFE=yes)
  *     use non-*_r functions if they are thread-safe
- *
- * One thread-safe solution for gethostbyname() might be to use getaddrinfo().
  */
 
+#ifndef WIN32
 
 /*
  * Wrapper around getpwuid() or getpwuid_r() to mimic POSIX getpwuid_r()
@@ -60,8 +59,7 @@
  * error during lookup: returns an errno code, *result is NULL
  * (caller should *not* assume that the errno variable is set)
  */
-#ifndef WIN32
-int
+static int
 pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer,
           size_t buflen, struct passwd **result)
 {
@@ -75,42 +73,74 @@ pqGetpwuid(uid_t uid, struct passwd *resultbuf, char *buffer,
    return (*result == NULL) ? errno : 0;
 #endif
 }
-#endif
 
 /*
- * Wrapper around gethostbyname() or gethostbyname_r() to mimic
- * POSIX gethostbyname_r() behaviour, if it is not available or required.
- * This function is called _only_ by our getaddrinfo() portability function.
+ * pg_get_user_name - get the name of the user with the given ID
+ *
+ * On success, the user name is returned into the buffer (of size buflen),
+ * and "true" is returned.  On failure, a localized error message is
+ * returned into the buffer, and "false" is returned.
  */
-#ifndef HAVE_GETADDRINFO
-int
-pqGethostbyname(const char *name,
-               struct hostent *resultbuf,
-               char *buffer, size_t buflen,
-               struct hostent **result,
-               int *herrno)
+bool
+pg_get_user_name(uid_t user_id, char *buffer, size_t buflen)
 {
-#if defined(FRONTEND) && defined(ENABLE_THREAD_SAFETY) && defined(HAVE_GETHOSTBYNAME_R)
-
-   /*
-    * broken (well early POSIX draft) gethostbyname_r() which returns 'struct
-    * hostent *'
-    */
-   *result = gethostbyname_r(name, resultbuf, buffer, buflen, herrno);
-   return (*result == NULL) ? -1 : 0;
-#else
+   char        pwdbuf[BUFSIZ];
+   struct passwd pwdstr;
+   struct passwd *pw = NULL;
+   int         pwerr;
 
-   /* no gethostbyname_r(), just use gethostbyname() */
-   *result = gethostbyname(name);
+   pwerr = pqGetpwuid(user_id, &pwdstr, pwdbuf, sizeof(pwdbuf), &pw);
+   if (pw != NULL)
+   {
+       strlcpy(buffer, pw->pw_name, buflen);
+       return true;
+   }
+   if (pwerr != 0)
+       snprintf(buffer, buflen,
+                _("could not look up local user ID %d: %s"),
+                (int) user_id,
+                strerror_r(pwerr, pwdbuf, sizeof(pwdbuf)));
+   else
+       snprintf(buffer, buflen,
+                _("local user with ID %d does not exist"),
+                (int) user_id);
+   return false;
+}
 
-   if (*result != NULL)
-       *herrno = h_errno;
+/*
+ * pg_get_user_home_dir - get the home directory of the user with the given ID
+ *
+ * On success, the directory path is returned into the buffer (of size buflen),
+ * and "true" is returned.  On failure, a localized error message is
+ * returned into the buffer, and "false" is returned.
+ *
+ * Note that this does not incorporate the common behavior of checking
+ * $HOME first, since it's independent of which user_id is queried.
+ */
+bool
+pg_get_user_home_dir(uid_t user_id, char *buffer, size_t buflen)
+{
+   char        pwdbuf[BUFSIZ];
+   struct passwd pwdstr;
+   struct passwd *pw = NULL;
+   int         pwerr;
 
-   if (*result != NULL)
-       return 0;
+   pwerr = pqGetpwuid(user_id, &pwdstr, pwdbuf, sizeof(pwdbuf), &pw);
+   if (pw != NULL)
+   {
+       strlcpy(buffer, pw->pw_dir, buflen);
+       return true;
+   }
+   if (pwerr != 0)
+       snprintf(buffer, buflen,
+                _("could not look up local user ID %d: %s"),
+                (int) user_id,
+                strerror_r(pwerr, pwdbuf, sizeof(pwdbuf)));
    else
-       return -1;
-#endif
+       snprintf(buffer, buflen,
+                _("local user with ID %d does not exist"),
+                (int) user_id);
+   return false;
 }
 
-#endif
+#endif                         /* !WIN32 */
index deb62c1c7b3a8f672e7e275ac2e3c5d3cee24a66..ec3546d0c0fa99f44afa169a2adb29b8691a371f 100644 (file)
@@ -106,7 +106,7 @@ sub mkvcbuild
      pread.c preadv.c pwrite.c pwritev.c pg_bitutils.c
      pg_strong_random.c pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c
      pqsignal.c mkdtemp.c qsort.c qsort_arg.c bsearch_arg.c quotes.c system.c
-     strerror.c tar.c thread.c
+     strerror.c tar.c
      win32env.c win32error.c win32ntdll.c
      win32security.c win32setlocale.c win32stat.c);