Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests - Mailing list pgsql-patches

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom Lane wrote:
> >> First line of thought: we surely must not insert a snprintf into
> >> libpq.so unless it is 100% up to spec *and* has no performance issues
> >> ... neither of which can be claimed of the CVS-tip version.
>
> > Agreed, and we have to support all the 64-bit specifications a port
> > might support like %qd and %I64d as well as %lld.  I have added that to
> > our current CVS version.
>
> I really dislike that idea and request that you revert it.
>
> > Is there any way we can have just gettext() call our snprintf under a
> > special name?
>
> The issue only comes up in libpq --- in the backend there is no reason
> that snprintf can't be our snprintf, and likewise in self-contained
> programs like psql.  It might be worth the pain-in-the-neck quality to
> have libpq refer to the functions as pq_snprintf etc.  Perhaps we could
> do this via macros
>
> #define snprintf    pq_snprintf
>
> and not have to uglify the source code.

OK, here is a patch that changes snprintf() to pg_snprintf() for
platforms that need our snprintf.  We do the same thing with
pgrename/pgunlink.

It manages to preserve printf-like argument checking in gcc.

I considered using a macro just in libpq but realized it was going to be
too ugly and too easy to break without us detecting it.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: configure
===================================================================
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.432
diff -c -c -r1.432 configure
*** configure    2 Mar 2005 15:42:35 -0000    1.432
--- configure    10 Mar 2005 21:05:27 -0000
***************
*** 14973,14978 ****
--- 14973,14983 ----

  # Now we have checked all the reasons to replace snprintf
  if test $pgac_need_repl_snprintf = yes; then
+
+ cat >>confdefs.h <<\_ACEOF
+ #define USE_SNPRINTF 1
+ _ACEOF
+
    LIBOBJS="$LIBOBJS snprintf.$ac_objext"
  fi

Index: configure.in
===================================================================
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.405
diff -c -c -r1.405 configure.in
*** configure.in    2 Mar 2005 15:42:35 -0000    1.405
--- configure.in    10 Mar 2005 21:05:28 -0000
***************
*** 1143,1148 ****
--- 1143,1149 ----

  # Now we have checked all the reasons to replace snprintf
  if test $pgac_need_repl_snprintf = yes; then
+   AC_DEFINE(USE_SNPRINTF, 1, [Use replacement snprintf() functions.])
    AC_LIBOBJ(snprintf)
  fi

Index: src/bin/pg_ctl/pg_ctl.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.54
diff -c -c -r1.54 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c    22 Feb 2005 04:39:22 -0000    1.54
--- src/bin/pg_ctl/pg_ctl.c    10 Mar 2005 21:05:30 -0000
***************
*** 337,355 ****
      if (log_file != NULL)
  #ifndef WIN32    /* Cygwin doesn't have START */
          snprintf(cmd, MAXPGPATH, "%s\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1 &%s",
  #else
          snprintf(cmd, MAXPGPATH, "%sSTART /B \"\" \"%s\" %s%s < \"%s\" >> \"%s\" 2>&1%s",
- #endif
                   SYSTEMQUOTE, postgres_path, pgdata_opt, post_opts,
                   DEVNULL, log_file, SYSTEMQUOTE);
      else
  #ifndef WIN32    /* Cygwin doesn't have START */
          snprintf(cmd, MAXPGPATH, "%s\"%s\" %s%s < \"%s\" 2>&1 &%s",
  #else
          snprintf(cmd, MAXPGPATH, "%sSTART /B \"\" \"%s\" %s%s < \"%s\" 2>&1%s",
- #endif
                   SYSTEMQUOTE, postgres_path, pgdata_opt, post_opts,
                   DEVNULL, SYSTEMQUOTE);

      return system(cmd);
  }
--- 337,359 ----
      if (log_file != NULL)
  #ifndef WIN32    /* Cygwin doesn't have START */
          snprintf(cmd, MAXPGPATH, "%s\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1 &%s",
+                  SYSTEMQUOTE, postgres_path, pgdata_opt, post_opts,
+                  DEVNULL, log_file, SYSTEMQUOTE);
  #else
          snprintf(cmd, MAXPGPATH, "%sSTART /B \"\" \"%s\" %s%s < \"%s\" >> \"%s\" 2>&1%s",
                   SYSTEMQUOTE, postgres_path, pgdata_opt, post_opts,
                   DEVNULL, log_file, SYSTEMQUOTE);
+ #endif
      else
  #ifndef WIN32    /* Cygwin doesn't have START */
          snprintf(cmd, MAXPGPATH, "%s\"%s\" %s%s < \"%s\" 2>&1 &%s",
+                  SYSTEMQUOTE, postgres_path, pgdata_opt, post_opts,
+                  DEVNULL, SYSTEMQUOTE);
  #else
          snprintf(cmd, MAXPGPATH, "%sSTART /B \"\" \"%s\" %s%s < \"%s\" 2>&1%s",
                   SYSTEMQUOTE, postgres_path, pgdata_opt, post_opts,
                   DEVNULL, SYSTEMQUOTE);
+ #endif

      return system(cmd);
  }
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.140
diff -c -c -r1.140 command.c
*** src/bin/psql/command.c    22 Feb 2005 04:40:51 -0000    1.140
--- src/bin/psql/command.c    10 Mar 2005 21:05:31 -0000
***************
*** 1175,1187 ****
           *    supplied path unless we use only backslashes, so we do that.
           */
  #endif
-         snprintf(fnametmp, sizeof(fnametmp), "%s%spsql.edit.%d", tmpdir,
  #ifndef WIN32
!                 "/",
  #else
!                 "",    /* trailing separator already present */
  #endif
-                 (int)getpid());

          fname = (const char *) fnametmp;

--- 1175,1187 ----
           *    supplied path unless we use only backslashes, so we do that.
           */
  #endif
  #ifndef WIN32
!         snprintf(fnametmp, sizeof(fnametmp), "%s%spsql.edit.%d", tmpdir,
!                 "/", (int)getpid());
  #else
!         snprintf(fnametmp, sizeof(fnametmp), "%s%spsql.edit.%d", tmpdir,
!                 "" /* trailing separator already present */, (int)getpid());
  #endif

          fname = (const char *) fnametmp;

Index: src/include/pg_config.h.in
===================================================================
RCS file: /cvsroot/pgsql/src/include/pg_config.h.in,v
retrieving revision 1.81
diff -c -c -r1.81 pg_config.h.in
*** src/include/pg_config.h.in    6 Nov 2004 23:06:25 -0000    1.81
--- src/include/pg_config.h.in    10 Mar 2005 21:05:31 -0000
***************
*** 648,653 ****
--- 648,656 ----
  /* Define to 1 to build with Rendezvous support. (--with-rendezvous) */
  #undef USE_RENDEZVOUS

+ /* Use replacement snprintf() functions. */
+ #undef USE_SNPRINTF
+
  /* Define to build with (Open)SSL support. (--with-openssl) */
  #undef USE_SSL

Index: src/include/port.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/port.h,v
retrieving revision 1.70
diff -c -c -r1.70 port.h
*** src/include/port.h    27 Feb 2005 00:53:29 -0000    1.70
--- src/include/port.h    10 Mar 2005 21:05:31 -0000
***************
*** 107,112 ****
--- 107,132 ----
  extern unsigned char pg_toupper(unsigned char ch);
  extern unsigned char pg_tolower(unsigned char ch);

+ #ifdef USE_SNPRINTF
+ extern int pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args);
+ extern int pg_snprintf(char *str, size_t count, const char *fmt,...)
+ /* This extension allows gcc to check the format string */
+ __attribute__((format(printf, 3, 4)));
+ extern int pg_printf(const char *fmt,...)
+ /* This extension allows gcc to check the format string */
+ __attribute__((format(printf, 1, 2)));
+
+ #ifdef __GNUC__
+ #define vsnprintf(...)    pg_vsnprintf(__VA_ARGS__)
+ #define snprintf(...)    pg_snprintf(__VA_ARGS__)
+ #define printf(...)        pg_printf(__VA_ARGS__)
+ #else
+ #define vsnprintf        pg_vsnprintf
+ #define snprintf        pg_snprintf
+ #define printf            pg_printf
+ #endif
+ #endif
+
  /* Portable prompt handling */
  extern char *simple_prompt(const char *prompt, int maxlen, bool echo);

Index: src/port/snprintf.c
===================================================================
RCS file: /cvsroot/pgsql/src/port/snprintf.c,v
retrieving revision 1.16
diff -c -c -r1.16 snprintf.c
*** src/port/snprintf.c    2 Mar 2005 23:56:53 -0000    1.16
--- src/port/snprintf.c    10 Mar 2005 21:05:32 -0000
***************
*** 67,83 ****

  /*static char _id[] = "$PostgreSQL: pgsql/src/port/snprintf.c,v 1.16 2005/03/02 23:56:53 momjian Exp $";*/

! int            snprintf(char *str, size_t count, const char *fmt,...);
! int            vsnprintf(char *str, size_t count, const char *fmt, va_list args);
! int            printf(const char *format, ...);
  static void dopr(char *buffer, const char *format, va_list args, char *end);

- /*
-  *    If vsnprintf() is not before snprintf() in this file, snprintf()
-  *    will call the system vsnprintf() on MinGW.
-  */
  int
! vsnprintf(char *str, size_t count, const char *fmt, va_list args)
  {
      char *end;
      str[0] = '\0';
--- 67,79 ----

  /*static char _id[] = "$PostgreSQL: pgsql/src/port/snprintf.c,v 1.16 2005/03/02 23:56:53 momjian Exp $";*/

! int            pg_snprintf(char *str, size_t count, const char *fmt,...);
! int            pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args);
! int            pg_printf(const char *format, ...);
  static void dopr(char *buffer, const char *format, va_list args, char *end);

  int
! pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args)
  {
      char *end;
      str[0] = '\0';
***************
*** 89,107 ****
  }

  int
! snprintf(char *str, size_t count, const char *fmt,...)
  {
      int            len;
      va_list        args;

      va_start(args, fmt);
!     len = vsnprintf(str, count, fmt, args);
      va_end(args);
      return len;
  }

  int
! printf(const char *fmt,...)
  {
      int            len;
      va_list            args;
--- 85,103 ----
  }

  int
! pg_snprintf(char *str, size_t count, const char *fmt,...)
  {
      int            len;
      va_list        args;

      va_start(args, fmt);
!     len = pg_vsnprintf(str, count, fmt, args);
      va_end(args);
      return len;
  }

  int
! pg_printf(const char *fmt,...)
  {
      int            len;
      va_list            args;
***************
*** 109,115 ****
      char*            p;

      va_start(args, fmt);
!     len = vsnprintf((char*)buffer, (size_t)4096, fmt, args);
      va_end(args);
      p = (char*)buffer;
      for(;*p;p++)
--- 105,111 ----
      char*            p;

      va_start(args, fmt);
!     len = pg_vsnprintf((char*)buffer, (size_t)4096, fmt, args);
      va_end(args);
      p = (char*)buffer;
      for(;*p;p++)