Re: Patch to change psql default banner v6 - Mailing list pgsql-patches

David Fetter wrote:
> On Fri, May 16, 2008 at 01:22:55AM -0400, Tom Lane wrote:
> > David Fetter <david@fetter.org> writes:
> > > I believe there's a bug in this patch, namely that the warnings when
> > > there's a server-client mismatch only appear at startup time.
> >
> > Please do not blame this patch for a problem that has been there all
> > along.
> >
> > I don't say that the point doesn't need investigation, but blaming
> > the patch-at-hand for the issue is just misleading.
>
> The patch at hand, as you point out, emphasizes a problem that's been
> there all along, namely that \c doesn't do the same things that
> command line connection does.
>
> I'm volunteering to make them use the same methods :)

David, I have fixed this problem with the attached, applied patch.
Thanks for the observation.  (The patch also removes a duplicate
definition of parse_version().)

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.191
diff -c -c -r1.191 command.c
*** src/bin/psql/command.c    26 Jun 2008 01:35:45 -0000    1.191
--- src/bin/psql/command.c    30 Jun 2008 23:56:53 -0000
***************
*** 29,34 ****
--- 29,37 ----
  #include <sys/types.h>            /* for umask() */
  #include <sys/stat.h>            /* for stat() */
  #endif
+ #ifdef USE_SSL
+ #include <openssl/ssl.h>
+ #endif

  #include "portability/instr_time.h"

***************
*** 57,62 ****
--- 60,74 ----
  static bool do_connect(char *dbname, char *user, char *host, char *port);
  static bool do_shell(const char *command);

+ #ifdef USE_SSL
+ static void printSSLInfo(void);
+ #endif
+
+ #ifdef WIN32
+ static void checkWin32Codepage(void);
+ #endif
+
+

  /*----------
   * HandleSlashCmds:
***************
*** 1185,1190 ****
--- 1197,1203 ----
       * Replace the old connection with the new one, and update
       * connection-dependent variables.
       */
+     connection_warnings();
      PQsetNoticeProcessor(n_conn, NoticeProcessor, NULL);
      pset.db = n_conn;
      SyncVariables();
***************
*** 1212,1217 ****
--- 1225,1324 ----
  }


+ void
+ connection_warnings(void)
+ {
+     if (!pset.quiet && !pset.notty)
+     {
+         int            client_ver = parse_version(PG_VERSION);
+
+         if (pset.sversion != client_ver)
+         {
+             const char *server_version;
+             char        server_ver_str[16];
+
+             /* Try to get full text form, might include "devel" etc */
+             server_version = PQparameterStatus(pset.db, "server_version");
+             if (!server_version)
+             {
+                 snprintf(server_ver_str, sizeof(server_ver_str),
+                          "%d.%d.%d",
+                          pset.sversion / 10000,
+                          (pset.sversion / 100) % 100,
+                          pset.sversion % 100);
+                 server_version = server_ver_str;
+             }
+
+             printf(_("%s (%s, server %s)\n"),
+             pset.progname, PG_VERSION, server_version);
+         }
+         else
+             printf("%s (%s)\n", pset.progname, PG_VERSION);
+
+         if (pset.sversion / 100 != client_ver / 100)
+             printf(_("WARNING: %s version %d.%d, server version %d.%d.\n"
+                  "         Some psql features might not work.\n"),
+                 pset.progname, client_ver / 10000, (client_ver / 100) % 100,
+                 pset.sversion / 10000, (pset.sversion / 100) % 100);
+
+ #ifdef WIN32
+         checkWin32Codepage();
+ #endif
+ #ifdef USE_SSL
+         printSSLInfo();
+ #endif
+     }
+ }
+
+
+ /*
+  * printSSLInfo
+  *
+  * Prints information about the current SSL connection, if SSL is in use
+  */
+ #ifdef USE_SSL
+ static void
+ printSSLInfo(void)
+ {
+     int            sslbits = -1;
+     SSL           *ssl;
+
+     ssl = PQgetssl(pset.db);
+     if (!ssl)
+         return;                    /* no SSL */
+
+     SSL_get_cipher_bits(ssl, &sslbits);
+     printf(_("SSL connection (cipher: %s, bits: %i)\n"),
+            SSL_get_cipher(ssl), sslbits);
+ }
+ #endif
+
+
+ /*
+  * checkWin32Codepage
+  *
+  * Prints a warning when win32 console codepage differs from Windows codepage
+  */
+ #ifdef WIN32
+ static void
+ checkWin32Codepage(void)
+ {
+     unsigned int wincp,
+                 concp;
+
+     wincp = GetACP();
+     concp = GetConsoleCP();
+     if (wincp != concp)
+     {
+         printf(_("WARNING: Console code page (%u) differs from Windows code page (%u)\n"
+                  "         8-bit characters might not work correctly. See psql reference\n"
+                  "         page \"Notes for Windows users\" for details.\n"),
+                concp, wincp);
+     }
+ }
+ #endif
+
+
  /*
   * SyncVariables
   *
Index: src/bin/psql/command.h
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.h,v
retrieving revision 1.30
diff -c -c -r1.30 command.h
*** src/bin/psql/command.h    1 Jan 2008 19:45:55 -0000    1.30
--- src/bin/psql/command.h    30 Jun 2008 23:56:53 -0000
***************
*** 34,39 ****
--- 34,41 ----
          printQueryOpt *popt,
          bool quiet);

+ extern void connection_warnings(void);
+
  extern void SyncVariables(void);

  extern void UnsyncVariables(void);
Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.148
diff -c -c -r1.148 startup.c
*** src/bin/psql/startup.c    16 May 2008 17:17:00 -0000    1.148
--- src/bin/psql/startup.c    30 Jun 2008 23:56:53 -0000
***************
*** 8,16 ****
  #include "postgres_fe.h"

  #include <sys/types.h>
- #ifdef USE_SSL
- #include <openssl/ssl.h>
- #endif

  #ifndef WIN32
  #include <unistd.h>
--- 8,13 ----
***************
*** 78,84 ****
      bool        single_txn;
  };

- static int    parse_version(const char *versionString);
  static void parse_psql_options(int argc, char *argv[],
                     struct adhoc_opts * options);
  static void process_psqlrc(char *argv0);
--- 75,80 ----
***************
*** 86,99 ****
  static void showVersion(void);
  static void EstablishVariableSpace(void);

- #ifdef USE_SSL
- static void printSSLInfo(void);
- #endif
-
- #ifdef WIN32
- static void checkWin32Codepage(void);
- #endif
-
  /*
   *
   * main
--- 82,87 ----
***************
*** 296,344 ****
          if (!options.no_psqlrc)
              process_psqlrc(argv[0]);

          if (!pset.quiet && !pset.notty)
-         {
-             int            client_ver = parse_version(PG_VERSION);
-
-             if (pset.sversion != client_ver)
-             {
-                 const char *server_version;
-                 char        server_ver_str[16];
-
-                 /* Try to get full text form, might include "devel" etc */
-                 server_version = PQparameterStatus(pset.db, "server_version");
-                 if (!server_version)
-                 {
-                     snprintf(server_ver_str, sizeof(server_ver_str),
-                              "%d.%d.%d",
-                              pset.sversion / 10000,
-                              (pset.sversion / 100) % 100,
-                              pset.sversion % 100);
-                     server_version = server_ver_str;
-                 }
-
-                 printf(_("%s (%s, server %s)\n"),
-                 pset.progname, PG_VERSION, server_version);
-             }
-             else
-                 printf("%s (%s)\n", pset.progname, PG_VERSION);
-
-             if (pset.sversion / 100 != client_ver / 100)
-                 printf(_("WARNING: %s version %d.%d, server version %d.%d.\n"
-                      "         Some psql features might not work.\n"),
-                     pset.progname, client_ver / 10000, (client_ver / 100) % 100,
-                     pset.sversion / 10000, (pset.sversion / 100) % 100);
-
- #ifdef WIN32
-             checkWin32Codepage();
- #endif
- #ifdef USE_SSL
-             printSSLInfo();
- #endif
-
              printf(_("Type \"help\" for help.\n\n"));
-         }
-
          if (!pset.notty)
              initializeInput(options.no_readline ? 0 : 1);
          if (options.action_string)        /* -f - was used */
--- 284,292 ----
          if (!options.no_psqlrc)
              process_psqlrc(argv[0]);

+         connection_warnings();
          if (!pset.quiet && !pset.notty)
              printf(_("Type \"help\" for help.\n\n"));
          if (!pset.notty)
              initializeInput(options.no_readline ? 0 : 1);
          if (options.action_string)        /* -f - was used */
***************
*** 358,386 ****


  /*
-  * Convert a version string into a number.
-  */
- static int
- parse_version(const char *versionString)
- {
-     int            cnt;
-     int            vmaj,
-                 vmin,
-                 vrev;
-
-     cnt = sscanf(versionString, "%d.%d.%d", &vmaj, &vmin, &vrev);
-
-     if (cnt < 2)
-         return -1;
-
-     if (cnt == 2)
-         vrev = 0;
-
-     return (100 * vmaj + vmin) * 100 + vrev;
- }
-
-
- /*
   * Parse command line options
   */

--- 306,311 ----
***************
*** 684,737 ****


  /*
-  * printSSLInfo
-  *
-  * Prints information about the current SSL connection, if SSL is in use
-  */
- #ifdef USE_SSL
- static void
- printSSLInfo(void)
- {
-     int            sslbits = -1;
-     SSL           *ssl;
-
-     ssl = PQgetssl(pset.db);
-     if (!ssl)
-         return;                    /* no SSL */
-
-     SSL_get_cipher_bits(ssl, &sslbits);
-     printf(_("SSL connection (cipher: %s, bits: %i)\n"),
-            SSL_get_cipher(ssl), sslbits);
- }
- #endif
-
-
- /*
-  * checkWin32Codepage
-  *
-  * Prints a warning when win32 console codepage differs from Windows codepage
-  */
- #ifdef WIN32
- static void
- checkWin32Codepage(void)
- {
-     unsigned int wincp,
-                 concp;
-
-     wincp = GetACP();
-     concp = GetConsoleCP();
-     if (wincp != concp)
-     {
-         printf(_("WARNING: Console code page (%u) differs from Windows code page (%u)\n"
-                  "         8-bit characters might not work correctly. See psql reference\n"
-                  "         page \"Notes for Windows users\" for details.\n"),
-                concp, wincp);
-     }
- }
- #endif
-
-
- /*
   * Assign hooks for psql variables.
   *
   * This isn't an amazingly good place for them, but neither is anywhere else.
--- 609,614 ----