Thread: [PATCH] Add use of asprintf()

There is a lot of code around that does something like

    char *val = malloc(...obscure computation...);
    sprintf(val, "...", ...);

This can be had simpler, with asprintf().  It is not standard, but it's
more widely accepted in C libraries than strlcpy for example.  The above
code would simply become

    char * val;
    asprintf(&val, "...", ...);

and asprintf() will figure out how much memory it needs by itself.

The attached patch should speak for itself.

I have supplied a few variants:

- asprintf() is the standard function, supplied by libpgport if
necessary.

- pg_asprintf() is asprintf() with automatic error handling (like
pg_malloc(), etc.)

- psprintf() is the same idea but with palloc.

I didn't touch most places involving MAXPGPATH.  There was a discussion
recently about reducing its usage to save memory.  That would work well
on top of my patch.

I did some desultory performance testing, which didn't result in any
concerns.  Most of the changes are not in performance-critical paths.

If you're very picky you might notice that the psprintf() implementation
asserts that vsnprintf() does not return negative results.  Very old
versions of glibc did that (before 2.1/February 1999), and this issue is
referenced in the current code of copy.c (via be4b8a86).  But I think
this issue is too ancient now to be of concern.

>From 60327b6d1810f49d6dd02ef87e2033b0ecafb34a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Fri, 13 Sep 2013 22:20:51 -0400
Subject: [PATCH] Add use of asprintf()

Add asprintf(), pg_asprintf(), and psprintf() to simply string
allocation and composition.  Replacement implementations taken from
NetBSD.
---
 configure                                          |   3 +-
 configure.in                                       |   2 +-
 contrib/adminpack/adminpack.c                      |   3 +-
 contrib/oid2name/oid2name.c                        |   3 +-
 contrib/pg_upgrade/check.c                         |  21 ++--
 contrib/pg_upgrade/tablespace.c                    |   7 +-
 contrib/pg_upgrade/util.c                          |   5 +-
 contrib/pg_upgrade/version_old_8_3.c               |   4 +-
 contrib/spi/refint.c                               |   3 +-
 contrib/spi/timetravel.c                           |   3 +-
 src/backend/bootstrap/bootstrap.c                  |   4 +-
 src/backend/catalog/catalog.c                      |  20 +---
 src/backend/commands/tablespace.c                  |  16 ++-
 src/backend/libpq/auth.c                           |  21 +---
 src/backend/optimizer/plan/subselect.c             |   3 +-
 src/backend/parser/gram.y                          |  33 +-----
 src/backend/postmaster/postmaster.c                |   7 +-
 src/backend/utils/adt/cash.c                       |  22 ++--
 src/backend/utils/adt/dbsize.c                     |  28 ++----
 src/backend/utils/adt/misc.c                       |  13 +--
 src/backend/utils/fmgr/dfmgr.c                     |  12 +--
 src/backend/utils/fmgr/fmgr.c                      |   5 +-
 src/backend/utils/init/miscinit.c                  |   8 +-
 src/backend/utils/misc/guc.c                       |   3 +-
 src/backend/utils/mmgr/mcxt.c                      |  49 +++++++++
 src/bin/initdb/initdb.c                            |  31 ++----
 src/bin/pg_ctl/pg_ctl.c                            |  16 +--
 src/bin/pg_dump/compress_io.c                      |  10 +-
 src/bin/pg_dump/pg_dump.c                          |   6 +-
 src/bin/psql/command.c                             |  40 +++-----
 src/bin/psql/common.c                              |   9 +-
 src/bin/psql/copy.c                                |   4 +-
 src/bin/psql/input.c                               |   6 +-
 src/bin/psql/large_obj.c                           |   4 +-
 src/bin/psql/startup.c                             |  14 +--
 src/bin/psql/tab-complete.c                        |   4 +-
 src/common/fe_memutils.c                           |  39 ++++++++
 src/common/relpath.c                               |  38 ++-----
 src/include/common/fe_memutils.h                   |   1 +
 src/include/pg_config.h.in                         |   3 +
 src/include/port.h                                 |   5 +
 src/include/utils/palloc.h                         |   2 +
 .../ecpg/test/expected/pgtypeslib-dt_test2.c       |   9 +-
 src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc   |   9 +-
 src/interfaces/libpq/fe-auth.c                     |  17 ++--
 src/port/asprintf.c                                | 111 +++++++++++++++++++++
 src/test/isolation/isolationtester.c               |  10 +-
 src/test/regress/pg_regress.c                      |  37 +++----
 48 files changed, 361 insertions(+), 362 deletions(-)
 create mode 100644 src/port/asprintf.c

diff --git a/configure b/configure
index c685ca3..3015d7e 100755
--- a/configure
+++ b/configure
@@ -21326,7 +21326,8 @@ fi



-for ac_func in crypt fls getopt getrusage inet_aton random rint srandom strerror strlcat strlcpy
+
+for ac_func in asprintf crypt fls getopt getrusage inet_aton random rint srandom strerror strlcat strlcpy
 do
 as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 { $as_echo "$as_me:$LINENO: checking for $ac_func" >&5
diff --git a/configure.in b/configure.in
index 82771bd..9a1c492 100644
--- a/configure.in
+++ b/configure.in
@@ -1344,7 +1344,7 @@ else
   AC_CHECK_FUNCS([fpclass fp_class fp_class_d class], [break])
 fi

-AC_REPLACE_FUNCS([crypt fls getopt getrusage inet_aton random rint srandom strerror strlcat strlcpy])
+AC_REPLACE_FUNCS([asprintf crypt fls getopt getrusage inet_aton random rint srandom strerror strlcat strlcpy])

 case $host_os in

diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c
index ded89c6..1ec0768 100644
--- a/contrib/adminpack/adminpack.c
+++ b/contrib/adminpack/adminpack.c
@@ -376,8 +376,7 @@
         /* Seems the timestamp is OK; prepare and return tuple */

         values[0] = timestampbuf;
-        values[1] = palloc(strlen(fctx->location) + strlen(de->d_name) + 2);
-        sprintf(values[1], "%s/%s", fctx->location, de->d_name);
+        values[1] = psprintf("%s/%s", fctx->location, de->d_name);

         tuple = BuildTupleFromCStrings(funcctx->attinmeta, values);

diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index cdec942..ab92c63 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -508,8 +508,7 @@ struct options
     free(comma_filenodes);

     /* now build the query */
-    todo = (char *) pg_malloc(650 + strlen(qualifiers));
-    snprintf(todo, 650 + strlen(qualifiers),
+    pg_asprintf(&todo,
              "SELECT pg_catalog.pg_relation_filenode(c.oid) as \"Filenode\", relname as \"Table Name\" %s\n"
              "FROM pg_catalog.pg_class c \n"
          "    LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace \n"
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 0376fcb..9b5293e 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -461,18 +461,13 @@ static void check_locale_and_encoding(ControlData *oldctrl,
     FILE       *script = NULL;
     char       *user_specification = "";

-    if (os_info.user_specified)
-    {
-        user_specification = pg_malloc(strlen(os_info.user) + 7);
-        sprintf(user_specification, "-U \"%s\" ", os_info.user);
-    }
-
-    *analyze_script_file_name = pg_malloc(MAXPGPATH);
-
     prep_status("Creating script to analyze new cluster");

-    snprintf(*analyze_script_file_name, MAXPGPATH, "analyze_new_cluster.%s",
-             SCRIPT_EXT);
+    if (os_info.user_specified)
+        pg_asprintf(&user_specification, "-U \"%s\" ", os_info.user);
+
+    pg_asprintf(analyze_script_file_name, "analyze_new_cluster.%s",
+                SCRIPT_EXT);

     if ((script = fopen_priv(*analyze_script_file_name, "w")) == NULL)
         pg_log(PG_FATAL, "Could not open file \"%s\": %s\n",
@@ -603,10 +598,8 @@ static void check_locale_and_encoding(ControlData *oldctrl,
     int            tblnum;
     char        old_cluster_pgdata[MAXPGPATH];

-    *deletion_script_file_name = pg_malloc(MAXPGPATH);
-
-    snprintf(*deletion_script_file_name, MAXPGPATH, "delete_old_cluster.%s",
-             SCRIPT_EXT);
+    pg_asprintf(deletion_script_file_name, "delete_old_cluster.%s",
+                SCRIPT_EXT);

     /*
      * Some users (oddly) create tablespaces inside the cluster data
diff --git a/contrib/pg_upgrade/tablespace.c b/contrib/pg_upgrade/tablespace.c
index 4747e79..d2aca74 100644
--- a/contrib/pg_upgrade/tablespace.c
+++ b/contrib/pg_upgrade/tablespace.c
@@ -85,12 +85,9 @@
     else
     {
         /* This cluster has a version-specific subdirectory */
-        cluster->tablespace_suffix = pg_malloc(4 +
-                                         strlen(cluster->major_version_str) +
-                                               10 /* OIDCHARS */ + 1);

         /* The leading slash is needed to start a new directory. */
-        sprintf(cluster->tablespace_suffix, "/PG_%s_%d", cluster->major_version_str,
-                cluster->controldata.cat_ver);
+        pg_asprintf(&cluster->tablespace_suffix, "/PG_%s_%d",
+                    cluster->major_version_str,    cluster->controldata.cat_ver);
     }
 }
diff --git a/contrib/pg_upgrade/util.c b/contrib/pg_upgrade/util.c
index 4da7658..2371f79 100644
--- a/contrib/pg_upgrade/util.c
+++ b/contrib/pg_upgrade/util.c
@@ -255,10 +255,9 @@
     if (val)
     {
 #ifndef WIN32
-        char       *envstr = (char *) pg_malloc(strlen(var) +
-                                                strlen(val) + 2);
+        char       *envstr;

-        sprintf(envstr, "%s=%s", var, val);
+        pg_asprintf(&envstr, "%s=%s", var, val);
         putenv(envstr);

         /*
diff --git a/contrib/pg_upgrade/version_old_8_3.c b/contrib/pg_upgrade/version_old_8_3.c
index e244dcf..ca211dd 100644
--- a/contrib/pg_upgrade/version_old_8_3.c
+++ b/contrib/pg_upgrade/version_old_8_3.c
@@ -678,9 +678,9 @@
     int            dbnum;
     FILE       *script = NULL;
     bool        found = false;
-    char       *output_path = pg_malloc(MAXPGPATH);
+    char       *output_path;

-    snprintf(output_path, MAXPGPATH, "adjust_sequences.sql");
+    output_path = pg_strdup("adjust_sequences.sql");

     prep_status("Creating script to adjust sequences");

diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index 8dc565a..fbed300 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -634,8 +634,7 @@
         (*nplans) = i = 0;
     }

-    newp->ident = (char *) malloc(strlen(ident) + 1);
-    strcpy(newp->ident, ident);
+    newp->ident = strdup(ident);
     newp->nplans = 0;
     newp->splan = NULL;
     (*nplans)++;
diff --git a/contrib/spi/timetravel.c b/contrib/spi/timetravel.c
index 34b4453..fa74dab 100644
--- a/contrib/spi/timetravel.c
+++ b/contrib/spi/timetravel.c
@@ -540,8 +540,7 @@
         (*nplans) = i = 0;
     }

-    newp->ident = (char *) malloc(strlen(ident) + 1);
-    strcpy(newp->ident, ident);
+    newp->ident = strdup(ident);
     newp->splan = NULL;
     (*nplans)++;

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index d23dc45..2a1af63 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -249,9 +249,9 @@ struct typmap
             case 'd':
                 {
                     /* Turn on debugging for the bootstrap process. */
-                    char       *debugstr = palloc(strlen("debug") + strlen(optarg) + 1);
+                    char       *debugstr;

-                    sprintf(debugstr, "debug%s", optarg);
+                    debugstr = psprintf("debug%s", optarg);
                     SetConfigOption("log_min_messages", debugstr,
                                     PGC_POSTMASTER, PGC_S_ARGV);
                     SetConfigOption("client_min_messages", debugstr,
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index c1287a7..577206c 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -75,35 +75,23 @@
 char *
 GetDatabasePath(Oid dbNode, Oid spcNode)
 {
-    int            pathlen;
-    char       *path;
-
     if (spcNode == GLOBALTABLESPACE_OID)
     {
         /* Shared system relations live in {datadir}/global */
         Assert(dbNode == 0);
-        pathlen = 6 + 1;
-        path = (char *) palloc(pathlen);
-        snprintf(path, pathlen, "global");
+        return pstrdup("global");
     }
     else if (spcNode == DEFAULTTABLESPACE_OID)
     {
         /* The default tablespace is {datadir}/base */
-        pathlen = 5 + OIDCHARS + 1;
-        path = (char *) palloc(pathlen);
-        snprintf(path, pathlen, "base/%u",
-                 dbNode);
+        return psprintf("base/%u", dbNode);
     }
     else
     {
         /* All other tablespaces are accessed via symlinks */
-        pathlen = 9 + 1 + OIDCHARS + 1 + strlen(TABLESPACE_VERSION_DIRECTORY) +
-            1 + OIDCHARS + 1;
-        path = (char *) palloc(pathlen);
-        snprintf(path, pathlen, "pg_tblspc/%u/%s/%u",
-                 spcNode, TABLESPACE_VERSION_DIRECTORY, dbNode);
+        return psprintf("pg_tblspc/%u/%s/%u",
+                        spcNode, TABLESPACE_VERSION_DIRECTORY, dbNode);
     }
-    return path;
 }


diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 155eb7c..ddc8ec7 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -541,12 +541,11 @@ static void create_tablespace_directories(const char *location,
 static void
 create_tablespace_directories(const char *location, const Oid tablespaceoid)
 {
-    char       *linkloc = palloc(OIDCHARS + OIDCHARS + 1);
-    char       *location_with_version_dir = palloc(strlen(location) + 1 +
-                                   strlen(TABLESPACE_VERSION_DIRECTORY) + 1);
+    char       *linkloc;
+    char       *location_with_version_dir;

-    sprintf(linkloc, "pg_tblspc/%u", tablespaceoid);
-    sprintf(location_with_version_dir, "%s/%s", location,
+    linkloc = psprintf("pg_tblspc/%u", tablespaceoid);
+    location_with_version_dir = psprintf("%s/%s", location,
             TABLESPACE_VERSION_DIRECTORY);

     /*
@@ -652,9 +651,7 @@ static void create_tablespace_directories(const char *location,
     char       *subfile;
     struct stat st;

-    linkloc_with_version_dir = palloc(9 + 1 + OIDCHARS + 1 +
-                                      strlen(TABLESPACE_VERSION_DIRECTORY));
-    sprintf(linkloc_with_version_dir, "pg_tblspc/%u/%s", tablespaceoid,
+    linkloc_with_version_dir = psprintf("pg_tblspc/%u/%s", tablespaceoid,
             TABLESPACE_VERSION_DIRECTORY);

     /*
@@ -711,8 +708,7 @@ static void create_tablespace_directories(const char *location,
             strcmp(de->d_name, "..") == 0)
             continue;

-        subfile = palloc(strlen(linkloc_with_version_dir) + 1 + strlen(de->d_name) + 1);
-        sprintf(subfile, "%s/%s", linkloc_with_version_dir, de->d_name);
+        subfile = psprintf("%s/%s", linkloc_with_version_dir, de->d_name);

         /* This check is just to deliver a friendlier error message */
         if (!redo && !directory_is_empty(subfile))
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 5b6a71c..7e65d28 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -1015,17 +1015,15 @@ static int pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
          */
         if (getenv("KRB5_KTNAME") == NULL)
         {
-            size_t        kt_len = strlen(pg_krb_server_keyfile) + 14;
-            char       *kt_path = malloc(kt_len);
+            char       *kt_path;

-            if (!kt_path)
+            if (asprintf(&kt_path, "KRB5_KTNAME=%s", pg_krb_server_keyfile) < 0)
             {
                 ereport(LOG,
                         (errcode(ERRCODE_OUT_OF_MEMORY),
                          errmsg("out of memory")));
                 return STATUS_ERROR;
             }
-            snprintf(kt_path, kt_len, "KRB5_KTNAME=%s", pg_krb_server_keyfile);
             putenv(kt_path);
         }
     }
@@ -1488,8 +1486,7 @@ static int pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
         char       *namebuf;
         int            retval;

-        namebuf = palloc(strlen(accountname) + strlen(domainname) + 2);
-        sprintf(namebuf, "%s@%s", accountname, domainname);
+        namebuf = psprintf("%s@%s", accountname, domainname);
         retval = check_usermap(port->hba->usermap, port->user_name, namebuf, true);
         pfree(namebuf);
         return retval;
@@ -2209,8 +2206,7 @@ static int pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
         attributes[0] = port->hba->ldapsearchattribute ? port->hba->ldapsearchattribute : "uid";
         attributes[1] = NULL;

-        filter = palloc(strlen(attributes[0]) + strlen(port->user_name) + 4);
-        sprintf(filter, "(%s=%s)",
+        filter = psprintf("(%s=%s)",
                 attributes[0],
                 port->user_name);

@@ -2299,17 +2295,10 @@ static int pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
         }
     }
     else
-    {
-        fulluser = palloc((port->hba->ldapprefix ? strlen(port->hba->ldapprefix) : 0) +
-                          strlen(port->user_name) +
-                (port->hba->ldapsuffix ? strlen(port->hba->ldapsuffix) : 0) +
-                          1);
-
-        sprintf(fulluser, "%s%s%s",
+        fulluser = psprintf("%s%s%s",
                 port->hba->ldapprefix ? port->hba->ldapprefix : "",
                 port->user_name,
                 port->hba->ldapsuffix ? port->hba->ldapsuffix : "");
-    }

     r = ldap_simple_bind_s(ldap, fulluser, passwd);
     ldap_unbind(ldap);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index bafb080..0df70c4 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -1119,8 +1119,7 @@ static Bitmapset *finalize_plan(PlannerInfo *root,
         root->cte_plan_ids = lappend_int(root->cte_plan_ids, splan->plan_id);

         /* Label the subplan for EXPLAIN purposes */
-        splan->plan_name = palloc(4 + strlen(cte->ctename) + 1);
-        sprintf(splan->plan_name, "CTE %s", cte->ctename);
+        splan->plan_name = psprintf("CTE %s", cte->ctename);

         /* Lastly, fill in the cost estimates for use later */
         cost_subplan(root, splan, plan);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a9812af..6e82141 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -1450,10 +1450,7 @@ set_rest_more:    /* Generic SET syntaxes: */

 var_name:    ColId                                { $$ = $1; }
             | var_name '.' ColId
-                {
-                    $$ = palloc(strlen($1) + strlen($3) + 2);
-                    sprintf($$, "%s.%s", $1, $3);
-                }
+                { $$ = psprintf("%s.%s", $1, $3); }
         ;

 var_list:    var_value                                { $$ = list_make1($1); }
@@ -10320,15 +10317,7 @@ ConstCharacter:  CharacterWithLength
 CharacterWithLength:  character '(' Iconst ')' opt_charset
                 {
                     if (($5 != NULL) && (strcmp($5, "sql_text") != 0))
-                    {
-                        char *type;
-
-                        type = palloc(strlen($1) + 1 + strlen($5) + 1);
-                        strcpy(type, $1);
-                        strcat(type, "_");
-                        strcat(type, $5);
-                        $1 = type;
-                    }
+                        $1 = psprintf("%s_%s", $1, $5);

                     $$ = SystemTypeName($1);
                     $$->typmods = list_make1(makeIntConst($3, @3));
@@ -10339,15 +10328,7 @@ CharacterWithLength:  character '(' Iconst ')' opt_charset
 CharacterWithoutLength:     character opt_charset
                 {
                     if (($2 != NULL) && (strcmp($2, "sql_text") != 0))
-                    {
-                        char *type;
-
-                        type = palloc(strlen($1) + 1 + strlen($2) + 1);
-                        strcpy(type, $1);
-                        strcat(type, "_");
-                        strcat(type, $2);
-                        $1 = type;
-                    }
+                        $1 = psprintf("%s_%s", $1, $2);

                     $$ = SystemTypeName($1);

@@ -13332,13 +13313,7 @@ doNegateFloat(Value *v)
     if (*oldval == '-')
         v->val.str = oldval+1;    /* just strip the '-' */
     else
-    {
-        char   *newval = (char *) palloc(strlen(oldval) + 2);
-
-        *newval = '-';
-        strcpy(newval+1, oldval);
-        v->val.str = newval;
-    }
+        v->val.str = psprintf("-%s", oldval);
 }

 static Node *
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 01d2618..1c0d1d1 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1965,12 +1965,7 @@ static bool save_backend_variables(BackendParameters *param, Port *port,
         else
         {
             /* Append '@' and dbname */
-            char       *db_user;
-
-            db_user = palloc(strlen(port->user_name) +
-                             strlen(port->database_name) + 2);
-            sprintf(db_user, "%s@%s", port->user_name, port->database_name);
-            port->user_name = db_user;
+            port->user_name = psprintf("%s@%s", port->user_name, port->database_name);
         }
     }

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index 82551c5..0158758 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -377,18 +377,16 @@
      *        from the value.
      *----------
      */
-    result = palloc(strlen(bufptr) + strlen(csymbol) + strlen(signsymbol) + 4);
-
     switch (sign_posn)
     {
         case 0:
             if (cs_precedes)
-                sprintf(result, "(%s%s%s)",
+                result = psprintf("(%s%s%s)",
                         csymbol,
                         (sep_by_space == 1) ? " " : "",
                         bufptr);
             else
-                sprintf(result, "(%s%s%s)",
+                result = psprintf("(%s%s%s)",
                         bufptr,
                         (sep_by_space == 1) ? " " : "",
                         csymbol);
@@ -396,14 +394,14 @@
         case 1:
         default:
             if (cs_precedes)
-                sprintf(result, "%s%s%s%s%s",
+                result = psprintf("%s%s%s%s%s",
                         signsymbol,
                         (sep_by_space == 2) ? " " : "",
                         csymbol,
                         (sep_by_space == 1) ? " " : "",
                         bufptr);
             else
-                sprintf(result, "%s%s%s%s%s",
+                result = psprintf("%s%s%s%s%s",
                         signsymbol,
                         (sep_by_space == 2) ? " " : "",
                         bufptr,
@@ -412,14 +410,14 @@
             break;
         case 2:
             if (cs_precedes)
-                sprintf(result, "%s%s%s%s%s",
+                result = psprintf("%s%s%s%s%s",
                         csymbol,
                         (sep_by_space == 1) ? " " : "",
                         bufptr,
                         (sep_by_space == 2) ? " " : "",
                         signsymbol);
             else
-                sprintf(result, "%s%s%s%s%s",
+                result = psprintf("%s%s%s%s%s",
                         bufptr,
                         (sep_by_space == 1) ? " " : "",
                         csymbol,
@@ -428,14 +426,14 @@
             break;
         case 3:
             if (cs_precedes)
-                sprintf(result, "%s%s%s%s%s",
+                result = psprintf("%s%s%s%s%s",
                         signsymbol,
                         (sep_by_space == 2) ? " " : "",
                         csymbol,
                         (sep_by_space == 1) ? " " : "",
                         bufptr);
             else
-                sprintf(result, "%s%s%s%s%s",
+                result = psprintf("%s%s%s%s%s",
                         bufptr,
                         (sep_by_space == 1) ? " " : "",
                         signsymbol,
@@ -444,14 +442,14 @@
             break;
         case 4:
             if (cs_precedes)
-                sprintf(result, "%s%s%s%s%s",
+                result = psprintf("%s%s%s%s%s",
                         csymbol,
                         (sep_by_space == 2) ? " " : "",
                         signsymbol,
                         (sep_by_space == 1) ? " " : "",
                         bufptr);
             else
-                sprintf(result, "%s%s%s%s%s",
+                result = psprintf("%s%s%s%s%s",
                         bufptr,
                         (sep_by_space == 1) ? " " : "",
                         csymbol,
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 8684746..08ea457 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -627,18 +627,14 @@
     Numeric        size = PG_GETARG_NUMERIC(0);
     Numeric        limit,
                 limit2;
-    char       *buf,
-               *result;
+    char       *result;

     limit = int64_to_numeric(10 * 1024);
     limit2 = int64_to_numeric(10 * 1024 * 2 - 1);

     if (numeric_is_less(size, limit))
     {
-        buf = numeric_to_cstring(size);
-        result = palloc(strlen(buf) + 7);
-        strcpy(result, buf);
-        strcat(result, " bytes");
+        result = psprintf("%s bytes", numeric_to_cstring(size));
     }
     else
     {
@@ -650,10 +646,7 @@
         {
             /* size = (size + 1) / 2 */
             size = numeric_plus_one_over_two(size);
-            buf = numeric_to_cstring(size);
-            result = palloc(strlen(buf) + 4);
-            strcpy(result, buf);
-            strcat(result, " kB");
+            result = psprintf("%s kB", numeric_to_cstring(size));
         }
         else
         {
@@ -663,10 +656,7 @@
             {
                 /* size = (size + 1) / 2 */
                 size = numeric_plus_one_over_two(size);
-                buf = numeric_to_cstring(size);
-                result = palloc(strlen(buf) + 4);
-                strcpy(result, buf);
-                strcat(result, " MB");
+                result = psprintf("%s MB", numeric_to_cstring(size));
             }
             else
             {
@@ -677,10 +667,7 @@
                 {
                     /* size = (size + 1) / 2 */
                     size = numeric_plus_one_over_two(size);
-                    buf = numeric_to_cstring(size);
-                    result = palloc(strlen(buf) + 4);
-                    strcpy(result, buf);
-                    strcat(result, " GB");
+                    result = psprintf("%s GB", numeric_to_cstring(size));
                 }
                 else
                 {
@@ -688,10 +675,7 @@
                     size = numeric_shift_right(size, 10);
                     /* size = (size + 1) / 2 */
                     size = numeric_plus_one_over_two(size);
-                    buf = numeric_to_cstring(size);
-                    result = palloc(strlen(buf) + 4);
-                    strcpy(result, buf);
-                    strcat(result, " TB");
+                    result = psprintf("%s TB", numeric_to_cstring(size));
                 }
             }
         }
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index aecdcd0..63a9916 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -242,11 +242,6 @@

         fctx = palloc(sizeof(ts_db_fctx));

-        /*
-         * size = tablespace dirname length + dir sep char + oid + terminator
-         */
-        fctx->location = (char *) palloc(9 + 1 + OIDCHARS + 1 +
-                                   strlen(TABLESPACE_VERSION_DIRECTORY) + 1);
         if (tablespaceOid == GLOBALTABLESPACE_OID)
         {
             fctx->dirdesc = NULL;
@@ -256,9 +251,9 @@
         else
         {
             if (tablespaceOid == DEFAULTTABLESPACE_OID)
-                sprintf(fctx->location, "base");
+                fctx->location = psprintf("base");
             else
-                sprintf(fctx->location, "pg_tblspc/%u/%s", tablespaceOid,
+                fctx->location = psprintf("pg_tblspc/%u/%s", tablespaceOid,
                         TABLESPACE_VERSION_DIRECTORY);

             fctx->dirdesc = AllocateDir(fctx->location);
@@ -297,9 +292,7 @@

         /* if database subdir is empty, don't report tablespace as used */

-        /* size = path length + dir sep char + file name + terminator */
-        subdir = palloc(strlen(fctx->location) + 1 + strlen(de->d_name) + 1);
-        sprintf(subdir, "%s/%s", fctx->location, de->d_name);
+        subdir = psprintf("%s/%s", fctx->location, de->d_name);
         dirdesc = AllocateDir(subdir);
         while ((de = ReadDir(dirdesc, subdir)) != NULL)
         {
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index 562a7c9..2dd9f75 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -503,9 +503,7 @@ static void incompatible_module_error(const char *libname,
         pfree(full);
     }

-    new = palloc(strlen(name) + strlen(DLSUFFIX) + 1);
-    strcpy(new, name);
-    strcat(new, DLSUFFIX);
+    new = psprintf("%s%s", name, DLSUFFIX);

     if (!have_slash)
     {
@@ -554,7 +552,6 @@ static void incompatible_module_error(const char *libname,
 substitute_libpath_macro(const char *name)
 {
     const char *sep_ptr;
-    char       *ret;

     AssertArg(name != NULL);

@@ -572,12 +569,7 @@ static void incompatible_module_error(const char *libname,
                  errmsg("invalid macro name in dynamic library path: %s",
                         name)));

-    ret = palloc(strlen(pkglib_path) + strlen(sep_ptr) + 1);
-
-    strcpy(ret, pkglib_path);
-    strcat(ret, sep_ptr);
-
-    return ret;
+    return psprintf("%s%s", pkglib_path, sep_ptr);
 }


diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 42de04c..9246a00 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -448,10 +448,7 @@ static void record_C_func(HeapTuple procedureTuple,
     const Pg_finfo_record *inforec;
     static Pg_finfo_record default_inforec = {0};

-    /* Compute name of info func */
-    infofuncname = (char *) palloc(strlen(funcname) + 10);
-    strcpy(infofuncname, "pg_finfo_");
-    strcat(infofuncname, funcname);
+    infofuncname = psprintf("pg_finfo_%s", funcname);

     /* Try to look up the info function */
     infofunc = (PGFInfoFunction) lookup_external_function(filehandle,
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index ed514f6..381a629 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -165,12 +165,10 @@
             }
         }

-        new = malloc(strlen(buf) + strlen(path) + 2);
-        if (!new)
+        if (asprintf(&new, "%s/%s", buf, path) < 0)
             ereport(FATAL,
                     (errcode(ERRCODE_OUT_OF_MEMORY),
                      errmsg("out of memory")));
-        sprintf(new, "%s/%s", buf, path);
         free(buf);
     }
     else
@@ -1286,9 +1284,7 @@
         {
             char       *expanded;

-            expanded = palloc(strlen("$libdir/plugins/") + strlen(filename) + 1);
-            strcpy(expanded, "$libdir/plugins/");
-            strcat(expanded, filename);
+            expanded = psprintf("$libdir/plugins/%s", filename);
             pfree(filename);
             filename = expanded;
         }
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 7d297bc..132526c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7902,8 +7902,7 @@ struct config_generic **
         name = record->name;

     /* build new item for array */
-    newval = palloc(strlen(name) + 1 + strlen(value) + 1);
-    sprintf(newval, "%s=%s", name, value);
+    newval = psprintf("%s=%s", name, value);
     datum = CStringGetTextDatum(newval);

     if (array)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9574fd3..b7beb13 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -852,3 +852,52 @@
     out[len] = '\0';
     return out;
 }
+
+/*
+ * asprintf()-like functions around palloc, adapted from
+ * http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/pkgtools/libnbcompat/files/asprintf.c
+ */
+
+char *
+psprintf(const char *format, ...)
+{
+    va_list ap;
+    char   *retval;
+
+    va_start(ap, format);
+    retval = pvsprintf(format, ap);
+    va_end(ap);
+
+    return retval;
+}
+
+char *
+pvsprintf(const char *format, va_list ap)
+{
+    char *buf, *new_buf;
+    size_t len;
+    int retval;
+    va_list ap2;
+
+    len = 128;
+    buf = palloc(len);
+
+    va_copy(ap2, ap);
+    retval = vsnprintf(buf, len, format, ap);
+    Assert(retval >= 0);
+
+    if (retval < len)
+    {
+        new_buf = repalloc(buf, retval + 1);
+        va_end(ap2);
+        return new_buf;
+    }
+
+    len = (size_t)retval + 1;
+    pfree(buf);
+    buf = palloc(len);
+    retval = vsnprintf(buf, len, format, ap2);
+    va_end(ap2);
+    Assert(retval == len - 1);
+    return buf;
+}
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index f66f530..94385dd 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -942,13 +942,10 @@ struct tsearch_config_match
 {
     char       *path;

-    path = pg_malloc(strlen(pg_data) + 2 +
-                     (subdir == NULL ? 0 : strlen(subdir)));
-
-    if (subdir != NULL)
-        sprintf(path, "%s/%s", pg_data, subdir);
+    if (subdir)
+        pg_asprintf(&path, "%s/%s", pg_data, subdir);
     else
-        strcpy(path, pg_data);
+        path = pg_strdup(pg_data);

     if (pg_mkdir_p(path, S_IRWXU) == 0)
         return true;
@@ -966,8 +963,7 @@ struct tsearch_config_match
 static void
 set_input(char **dest, char *filename)
 {
-    *dest = pg_malloc(strlen(share_path) + strlen(filename) + 2);
-    sprintf(*dest, "%s/%s", share_path, filename);
+    pg_asprintf(dest, "%s/%s", share_path, filename);
 }

 /*
@@ -1021,15 +1017,9 @@ struct tsearch_config_match
     char       *path;

     if (extrapath == NULL)
-    {
-        path = pg_malloc(strlen(pg_data) + 12);
-        sprintf(path, "%s/PG_VERSION", pg_data);
-    }
+        pg_asprintf(&path, "%s/PG_VERSION", pg_data);
     else
-    {
-        path = pg_malloc(strlen(pg_data) + strlen(extrapath) + 13);
-        sprintf(path, "%s/%s/PG_VERSION", pg_data, extrapath);
-    }
+        pg_asprintf(&path, "%s/%s/PG_VERSION", pg_data, extrapath);

     if ((version_file = fopen(path, PG_BINARY_W)) == NULL)
     {
@@ -1057,8 +1047,7 @@ struct tsearch_config_match
     FILE       *conf_file;
     char       *path;

-    path = pg_malloc(strlen(pg_data) + 17);
-    sprintf(path, "%s/postgresql.conf", pg_data);
+    pg_asprintf(&path, "%s/postgresql.conf", pg_data);
     conf_file = fopen(path, PG_BINARY_W);
     if (conf_file == NULL)
     {
@@ -2900,8 +2889,7 @@ struct tsearch_config_match
      * need quotes otherwise on Windows because paths there are most likely to
      * have embedded spaces.
      */
-    pgdata_set_env = pg_malloc(8 + strlen(pg_data));
-    sprintf(pgdata_set_env, "PGDATA=%s", pg_data);
+    pg_asprintf(&pgdata_set_env, "PGDATA=%s", pg_data);
     putenv(pgdata_set_env);
 }

@@ -3295,8 +3283,7 @@ struct tsearch_config_match
         }

         /* form name of the place where the symlink must go */
-        linkloc = (char *) pg_malloc(strlen(pg_data) + 8 + 1);
-        sprintf(linkloc, "%s/pg_xlog", pg_data);
+        pg_asprintf(&linkloc, "%s/pg_xlog", pg_data);

 #ifdef HAVE_SYMLINK
         if (symlink(xlog_dir, linkloc) != 0)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index cf50481..be51dc6 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2045,12 +2045,11 @@
                 case 'D':
                     {
                         char       *pgdata_D;
-                        char       *env_var = pg_malloc(strlen(optarg) + 8);
+                        char       *env_var;

                         pgdata_D = pg_strdup(optarg);
                         canonicalize_path(pgdata_D);
-                        snprintf(env_var, strlen(optarg) + 8, "PGDATA=%s",
-                                 pgdata_D);
+                        pg_asprintf(&env_var, "PGDATA=%s", pgdata_D);
                         putenv(env_var);

                         /*
@@ -2058,10 +2057,7 @@
                          * variable but we do -D too for clearer postmaster
                          * 'ps' display
                          */
-                        pgdata_opt = pg_malloc(strlen(pgdata_D) + 7);
-                        snprintf(pgdata_opt, strlen(pgdata_D) + 7,
-                                 "-D \"%s\" ",
-                                 pgdata_D);
+                        pg_asprintf(&pgdata_opt,  "-D \"%s\" ", pgdata_D);
                         break;
                     }
                 case 'l':
@@ -2102,11 +2098,7 @@
                         register_username = pg_strdup(optarg);
                     else
                         /* Prepend .\ for local accounts */
-                    {
-                        register_username = pg_malloc(strlen(optarg) + 3);
-                        strcpy(register_username, ".\\");
-                        strcat(register_username, optarg);
-                    }
+                        pg_asprintf(®ister_username, ".\\%s", optarg);
                     break;
                 case 'w':
                     do_wait = true;
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index 1dd31fb..d859c8e 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -487,10 +487,9 @@ struct cfp
 #ifdef HAVE_LIBZ
         if (fp == NULL)
         {
-            int            fnamelen = strlen(path) + 4;
-            char       *fname = pg_malloc(fnamelen);
+            char       *fname;

-            snprintf(fname, fnamelen, "%s%s", path, ".gz");
+            pg_asprintf(&fname, "%s.gz", path);
             fp = cfopen(fname, mode, 1);
             free(fname);
         }
@@ -518,10 +517,9 @@ struct cfp
     else
     {
 #ifdef HAVE_LIBZ
-        int            fnamelen = strlen(path) + 4;
-        char       *fname = pg_malloc(fnamelen);
+        char       *fname;

-        snprintf(fname, fnamelen, "%s%s", path, ".gz");
+        pg_asprintf(&fname, "%s.gz", path);
         fp = cfopen(fname, mode, 1);
         free(fname);
 #else
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 57320cc..01c63b1 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -10439,8 +10439,7 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer,
         /* If not schema-qualified, don't need to add OPERATOR() */
         if (!sawdot)
             return name;
-        oname = pg_malloc(strlen(name) + 11);
-        sprintf(oname, "OPERATOR(%s)", name);
+        pg_asprintf(&oname, "OPERATOR(%s)", name);
         free(name);
         return oname;
     }
@@ -12754,8 +12753,7 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer,
                 char       *acltag;

                 attnamecopy = pg_strdup(fmtId(attname));
-                acltag = pg_malloc(strlen(tbinfo->dobj.name) + strlen(attname) + 2);
-                sprintf(acltag, "%s.%s", tbinfo->dobj.name, attname);
+                pg_asprintf(&acltag, "%s.%s", tbinfo->dobj.name, attname);
                 /* Column's GRANT type is always TABLE */
                 dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE",
                         namecopy, attnamecopy, acltag,
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 351e684..73ad458 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1173,10 +1173,9 @@ static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
         else
         {
             /* Set variable to the value of the next argument */
-            int            len = strlen(envvar) + strlen(envval) + 1;
-            char       *newval = pg_malloc(len + 1);
+            char       *newval;

-            snprintf(newval, len + 1, "%s=%s", envvar, envval);
+            pg_asprintf(&newval, "%s=%s", envvar, envval);
             putenv(newval);
             success = true;

@@ -1537,9 +1536,7 @@ static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
     {
         char       *prompt_text;

-        prompt_text = pg_malloc(strlen(username) + 100);
-        snprintf(prompt_text, strlen(username) + 100,
-                 _("Password for user %s: "), username);
+        pg_asprintf(&prompt_text, _("Password for user %s: "), username);
         result = simple_prompt(prompt_text, 100, false);
         free(prompt_text);
     }
@@ -1910,14 +1907,6 @@ static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
         }
     }

-    /* Allocate sufficient memory for command line. */
-    if (lineno > 0)
-        sys = pg_malloc(strlen(editorName)
-                        + strlen(editor_lineno_arg) + 10        /* for integer */
-                        + 1 + strlen(fname) + 10 + 1);
-    else
-        sys = pg_malloc(strlen(editorName) + strlen(fname) + 10 + 1);
-
     /*
      * On Unix the EDITOR value should *not* be quoted, since it might include
      * switches, eg, EDITOR="pico -t"; it's up to the user to put quotes in it
@@ -1927,18 +1916,18 @@ static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
      */
 #ifndef WIN32
     if (lineno > 0)
-        sprintf(sys, "exec %s %s%d '%s'",
-                editorName, editor_lineno_arg, lineno, fname);
+        pg_asprintf(&sys, "exec %s %s%d '%s'",
+                    editorName, editor_lineno_arg, lineno, fname);
     else
-        sprintf(sys, "exec %s '%s'",
-                editorName, fname);
+        pg_asprintf(&sys, "exec %s '%s'",
+                    editorName, fname);
 #else
     if (lineno > 0)
-        sprintf(sys, SYSTEMQUOTE "\"%s\" %s%d \"%s\"" SYSTEMQUOTE,
+        pg_asprintf(&sys, SYSTEMQUOTE "\"%s\" %s%d \"%s\"" SYSTEMQUOTE,
                 editorName, editor_lineno_arg, lineno, fname);
     else
-        sprintf(sys, SYSTEMQUOTE "\"%s\" \"%s\"" SYSTEMQUOTE,
-                editorName, fname);
+        pg_asprintf(&sys, SYSTEMQUOTE "\"%s\" \"%s\"" SYSTEMQUOTE,
+                    editorName, fname);
 #endif
     result = system(sys);
     if (result == -1)
@@ -2557,14 +2546,11 @@ static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
         if (shellName == NULL)
             shellName = DEFAULT_SHELL;

-        sys = pg_malloc(strlen(shellName) + 16);
-#ifndef WIN32
-        sprintf(sys,
         /* See EDITOR handling comment for an explanation */
-                "exec %s", shellName);
+#ifndef WIN32
+        pg_asprintf(&sys, "exec %s", shellName);
 #else
-        /* See EDITOR handling comment for an explanation */
-        sprintf(sys, SYSTEMQUOTE "\"%s\"" SYSTEMQUOTE, shellName);
+        pg_asprintf(&sys, SYSTEMQUOTE "\"%s\"" SYSTEMQUOTE, shellName);
 #endif
         result = system(sys);
         free(sys);
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 3dea92c..71f0c8a 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -594,9 +594,7 @@
             char       *value;

             /* concate prefix and column name */
-            varname = pg_malloc(strlen(pset.gset_prefix) + strlen(colname) + 1);
-            strcpy(varname, pset.gset_prefix);
-            strcat(varname, colname);
+            pg_asprintf(&varname, "%s%s", pset.gset_prefix, colname);

             if (!PQgetisnull(result, 0, i))
                 value = PQgetvalue(result, 0, i);
@@ -1687,10 +1685,7 @@
         {
             char       *newfn;

-            newfn = pg_malloc(strlen(home) + strlen(p) + 1);
-            strcpy(newfn, home);
-            strcat(newfn, p);
-
+            pg_asprintf(&newfn, "%s%s", home, p);
             free(fn);
             *filename = newfn;
         }
diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c
index 13123d6..6db063c 100644
--- a/src/bin/psql/copy.c
+++ b/src/bin/psql/copy.c
@@ -79,9 +79,7 @@ struct copy_options
 {
     char       *newvar;

-    newvar = pg_malloc(strlen(*var) + strlen(more) + 1);
-    strcpy(newvar, *var);
-    strcat(newvar, more);
+    pg_asprintf(&newvar, "%s%s", *var, more);
     free(*var);
     *var = newvar;
 }
diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c
index 07c9e89..f2b6e4e 100644
--- a/src/bin/psql/input.c
+++ b/src/bin/psql/input.c
@@ -298,11 +298,7 @@
         if (histfile == NULL)
         {
             if (get_home_path(home))
-            {
-                psql_history = pg_malloc(strlen(home) + 1 +
-                                         strlen(PSQLHISTORY) + 1);
-                snprintf(psql_history, MAXPGPATH, "%s/%s", home, PSQLHISTORY);
-            }
+                pg_asprintf(&psql_history, "%s/%s", home, PSQLHISTORY);
         }
         else
         {
diff --git a/src/bin/psql/large_obj.c b/src/bin/psql/large_obj.c
index faaecce..1c2315a 100644
--- a/src/bin/psql/large_obj.c
+++ b/src/bin/psql/large_obj.c
@@ -201,10 +201,8 @@
         char       *bufptr;
         size_t        slen = strlen(comment_arg);

-        cmdbuf = malloc(slen * 2 + 256);
-        if (!cmdbuf)
+        if (asprintf(&cmdbuf, "COMMENT ON LARGE OBJECT %u IS '", loid) < 0)
             return fail_lo_xact("\\lo_import", own_transaction);
-        sprintf(cmdbuf, "COMMENT ON LARGE OBJECT %u IS '", loid);
         bufptr = cmdbuf + strlen(cmdbuf);
         bufptr += PQescapeStringConn(pset.db, bufptr, comment_arg, slen, NULL);
         strcpy(bufptr, "'");
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index b2264c9..c350758 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -184,12 +184,8 @@ static void parse_psql_options(int argc, char *argv[],
     if (options.username == NULL)
         password_prompt = pg_strdup(_("Password: "));
     else
-    {
-        password_prompt = pg_malloc(strlen(_("Password for user %s: ")) - 2 +
-                                    strlen(options.username) + 1);
-        sprintf(password_prompt, _("Password for user %s: "),
-                options.username);
-    }
+        pg_asprintf(&password_prompt, _("Password for user %s: "),
+                    options.username);

     if (pset.getPassword == TRI_YES)
         password = simple_prompt(password_prompt, 100, false);
@@ -642,10 +638,8 @@ static void parse_psql_options(int argc, char *argv[],
 #define R_OK 4
 #endif

-    psqlrc_minor = pg_malloc(strlen(filename) + 1 + strlen(PG_VERSION) + 1);
-    sprintf(psqlrc_minor, "%s-%s", filename, PG_VERSION);
-    psqlrc_major = pg_malloc(strlen(filename) + 1 + strlen(PG_MAJORVERSION) + 1);
-    sprintf(psqlrc_major, "%s-%s", filename, PG_MAJORVERSION);
+    pg_asprintf(&psqlrc_minor, "%s-%s", filename, PG_VERSION);
+    pg_asprintf(&psqlrc_major, "%s-%s", filename, PG_MAJORVERSION);

     /* check for minor version first, then major, then no version */
     if (access(psqlrc_minor, R_OK) == 0)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b3de387..9577239 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3822,7 +3822,6 @@ static char **complete_from_variables(char *text,
 complete_from_variables(char *text, const char *prefix, const char *suffix)
 {
     char      **matches;
-    int            overhead = strlen(prefix) + strlen(suffix) + 1;
     char      **varnames;
     int            nvars = 0;
     int            maxvars = 100;
@@ -3847,8 +3846,7 @@ static char **complete_from_variables(char *text,
             }
         }

-        buffer = (char *) pg_malloc(strlen(ptr->name) + overhead);
-        sprintf(buffer, "%s%s%s", prefix, ptr->name, suffix);
+        pg_asprintf(&buffer, "%s%s%s", prefix, ptr->name, suffix);
         varnames[nvars++] = buffer;
     }

diff --git a/src/common/fe_memutils.c b/src/common/fe_memutils.c
index bfe79f8..e59633d 100644
--- a/src/common/fe_memutils.c
+++ b/src/common/fe_memutils.c
@@ -93,6 +93,25 @@
         free(ptr);
 }

+int
+pg_asprintf(char **ret, const char *format, ...)
+{
+    va_list        ap;
+    int            rc;
+
+    va_start(ap, format);
+    rc = vasprintf(ret, format, ap);
+    va_end(ap);
+
+    if (ret < 0)
+    {
+        fprintf(stderr, _("out of memory\n"));
+        exit(EXIT_FAILURE);
+    }
+
+    return rc;
+}
+
 /*
  * Frontend emulation of backend memory management functions.  Useful for
  * programs that compile backend files.
@@ -126,3 +145,23 @@
 {
     return pg_realloc(pointer, size);
 }
+
+char *
+psprintf(const char *format, ...)
+{
+    va_list     ap;
+    int         rc;
+    char       *ret;
+
+    va_start(ap, format);
+    rc = vasprintf(&ret, format, ap);
+    va_end(ap);
+
+    if (rc < 0)
+    {
+        fprintf(stderr, _("out of memory\n"));
+        exit(EXIT_FAILURE);
+    }
+
+    return ret;
+}
diff --git a/src/common/relpath.c b/src/common/relpath.c
index 52f6b75..737003a 100644
--- a/src/common/relpath.c
+++ b/src/common/relpath.c
@@ -74,7 +74,6 @@
 char *
 relpathbackend(RelFileNode rnode, BackendId backend, ForkNumber forknum)
 {
-    int            pathlen;
     char       *path;

     if (rnode.spcNode == GLOBALTABLESPACE_OID)
@@ -82,41 +81,33 @@
         /* Shared system relations live in {datadir}/global */
         Assert(rnode.dbNode == 0);
         Assert(backend == InvalidBackendId);
-        pathlen = 7 + OIDCHARS + 1 + FORKNAMECHARS + 1;
-        path = (char *) palloc(pathlen);
         if (forknum != MAIN_FORKNUM)
-            snprintf(path, pathlen, "global/%u_%s",
+            path = psprintf("global/%u_%s",
                      rnode.relNode, forkNames[forknum]);
         else
-            snprintf(path, pathlen, "global/%u", rnode.relNode);
+            path = psprintf("global/%u", rnode.relNode);
     }
     else if (rnode.spcNode == DEFAULTTABLESPACE_OID)
     {
         /* The default tablespace is {datadir}/base */
         if (backend == InvalidBackendId)
         {
-            pathlen = 5 + OIDCHARS + 1 + OIDCHARS + 1 + FORKNAMECHARS + 1;
-            path = (char *) palloc(pathlen);
             if (forknum != MAIN_FORKNUM)
-                snprintf(path, pathlen, "base/%u/%u_%s",
+                path = psprintf("base/%u/%u_%s",
                          rnode.dbNode, rnode.relNode,
                          forkNames[forknum]);
             else
-                snprintf(path, pathlen, "base/%u/%u",
+                path = psprintf("base/%u/%u",
                          rnode.dbNode, rnode.relNode);
         }
         else
         {
-            /* OIDCHARS will suffice for an integer, too */
-            pathlen = 5 + OIDCHARS + 2 + OIDCHARS + 1 + OIDCHARS + 1
-                + FORKNAMECHARS + 1;
-            path = (char *) palloc(pathlen);
             if (forknum != MAIN_FORKNUM)
-                snprintf(path, pathlen, "base/%u/t%d_%u_%s",
+                path = psprintf("base/%u/t%d_%u_%s",
                          rnode.dbNode, backend, rnode.relNode,
                          forkNames[forknum]);
             else
-                snprintf(path, pathlen, "base/%u/t%d_%u",
+                path = psprintf("base/%u/t%d_%u",
                          rnode.dbNode, backend, rnode.relNode);
         }
     }
@@ -125,34 +116,25 @@
         /* All other tablespaces are accessed via symlinks */
         if (backend == InvalidBackendId)
         {
-            pathlen = 9 + 1 + OIDCHARS + 1
-                + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 + OIDCHARS + 1
-                + OIDCHARS + 1 + FORKNAMECHARS + 1;
-            path = (char *) palloc(pathlen);
             if (forknum != MAIN_FORKNUM)
-                snprintf(path, pathlen, "pg_tblspc/%u/%s/%u/%u_%s",
+                path = psprintf("pg_tblspc/%u/%s/%u/%u_%s",
                          rnode.spcNode, TABLESPACE_VERSION_DIRECTORY,
                          rnode.dbNode, rnode.relNode,
                          forkNames[forknum]);
             else
-                snprintf(path, pathlen, "pg_tblspc/%u/%s/%u/%u",
+                path = psprintf("pg_tblspc/%u/%s/%u/%u",
                          rnode.spcNode, TABLESPACE_VERSION_DIRECTORY,
                          rnode.dbNode, rnode.relNode);
         }
         else
         {
-            /* OIDCHARS will suffice for an integer, too */
-            pathlen = 9 + 1 + OIDCHARS + 1
-                + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 + OIDCHARS + 2
-                + OIDCHARS + 1 + OIDCHARS + 1 + FORKNAMECHARS + 1;
-            path = (char *) palloc(pathlen);
             if (forknum != MAIN_FORKNUM)
-                snprintf(path, pathlen, "pg_tblspc/%u/%s/%u/t%d_%u_%s",
+                path = psprintf("pg_tblspc/%u/%s/%u/t%d_%u_%s",
                          rnode.spcNode, TABLESPACE_VERSION_DIRECTORY,
                          rnode.dbNode, backend, rnode.relNode,
                          forkNames[forknum]);
             else
-                snprintf(path, pathlen, "pg_tblspc/%u/%s/%u/t%d_%u",
+                path = psprintf("pg_tblspc/%u/%s/%u/t%d_%u",
                          rnode.spcNode, TABLESPACE_VERSION_DIRECTORY,
                          rnode.dbNode, backend, rnode.relNode);
         }
diff --git a/src/include/common/fe_memutils.h b/src/include/common/fe_memutils.h
index 82ed8cd..db4e710 100644
--- a/src/include/common/fe_memutils.h
+++ b/src/include/common/fe_memutils.h
@@ -14,6 +14,7 @@ extern void *pg_malloc(size_t size);
 extern void *pg_malloc0(size_t size);
 extern void *pg_realloc(void *pointer, size_t size);
 extern void pg_free(void *pointer);
+extern int pg_asprintf(char **ret, const char *format, ...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));

 #include "utils/palloc.h"

diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 8aabf3c..e73c9b7 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -87,6 +87,9 @@
 /* Define to 1 if you have the `append_history' function. */
 #undef HAVE_APPEND_HISTORY

+/* Define to 1 if you have the `asprintf' function. */
+#undef HAVE_ASPRINTF
+
 /* Define to 1 if you have the `cbrt' function. */
 #undef HAVE_CBRT

diff --git a/src/include/port.h b/src/include/port.h
index 5ef4b0a..0b9dfc8 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -404,6 +404,11 @@ extern double rint(double x);
 extern int    inet_aton(const char *cp, struct in_addr * addr);
 #endif

+#ifndef HAVE_ASPRINTF
+extern int asprintf(char **ret, const char *fmt, ...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
+extern int vasprintf(char **ret, const char *fmt, va_list ap) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0)));
+#endif
+
 #if !HAVE_DECL_STRLCAT
 extern size_t strlcat(char *dst, const char *src, size_t siz);
 #endif
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index 01e7db5..03ef87e 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -101,5 +101,7 @@ extern void *palloc(Size size);
 extern void *palloc0(Size size);
 extern void pfree(void *pointer);
 extern void *repalloc(void *pointer, Size size);
+extern char *psprintf(const char *format, ...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2)));
+extern char *pvsprintf(const char *format, va_list ap) __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 0)));

 #endif   /* PALLOC_H */
diff --git a/src/interfaces/ecpg/test/expected/pgtypeslib-dt_test2.c
b/src/interfaces/ecpg/test/expected/pgtypeslib-dt_test2.c
index d3ebb0e..8d716e5 100644
--- a/src/interfaces/ecpg/test/expected/pgtypeslib-dt_test2.c
+++ b/src/interfaces/ecpg/test/expected/pgtypeslib-dt_test2.c
@@ -127,12 +127,9 @@
         {
             for (j = 0; times[j]; j++)
             {
-                int length = strlen(dates[i])
-                        + 1
-                        + strlen(times[j])
-                        + 1;
-                char* t = malloc(length);
-                sprintf(t, "%s %s", dates[i], times[j]);
+                char* t;
+                if (asprintf(&t, "%s %s", dates[i], times[j]) < 0)
+                    abort();
                 ts1 = PGTYPEStimestamp_from_asc(t, NULL);
                 text = PGTYPEStimestamp_to_asc(ts1);
                 if (i != 19 || j != 3) /* timestamp as integer or double differ for this case */
diff --git a/src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc b/src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc
index 0edf012..2a1c4a6 100644
--- a/src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc
+++ b/src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc
@@ -92,12 +92,9 @@ main(void)
         {
             for (j = 0; times[j]; j++)
             {
-                int length = strlen(dates[i])
-                        + 1
-                        + strlen(times[j])
-                        + 1;
-                char* t = malloc(length);
-                sprintf(t, "%s %s", dates[i], times[j]);
+                char* t;
+                if (asprintf(&t, "%s %s", dates[i], times[j]) < 0)
+                    abort();
                 ts1 = PGTYPEStimestamp_from_asc(t, NULL);
                 text = PGTYPEStimestamp_to_asc(ts1);
                 if (i != 19 || j != 3) /* timestamp as integer or double differ for this case */
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 5666a6b..dfc9cfb 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -420,7 +420,6 @@ struct krb5_info
 {
     OM_uint32    maj_stat,
                 min_stat;
-    int            maxlen;
     gss_buffer_desc temp_gbuf;

     if (!(conn->pghost && conn->pghost[0] != '\0'))
@@ -441,10 +440,14 @@ struct krb5_info
      * Import service principal name so the proper ticket can be acquired by
      * the GSSAPI system.
      */
-    maxlen = NI_MAXHOST + strlen(conn->krbsrvname) + 2;
-    temp_gbuf.value = (char *) malloc(maxlen);
-    snprintf(temp_gbuf.value, maxlen, "%s@%s",
-             conn->krbsrvname, conn->pghost);
+    if (asprintf((char **)&temp_gbuf.value, "%s@%s",
+                 conn->krbsrvname, conn->pghost) < 0)
+    {
+        printfPQExpBuffer(&conn->errorMessage,
+                          libpq_gettext("out of memory"));
+        return STATUS_ERROR;
+    }
+
     temp_gbuf.length = strlen(temp_gbuf.value);

     maj_stat = gss_import_name(&min_stat, &temp_gbuf,
@@ -656,13 +659,11 @@ struct krb5_info
                           libpq_gettext("host name must be specified\n"));
         return STATUS_ERROR;
     }
-    conn->sspitarget = malloc(strlen(conn->krbsrvname) + strlen(conn->pghost) + 2);
-    if (!conn->sspitarget)
+    if (asprintf(&conn->sspitarget, "%s/%s", conn->krbsrvname, conn->pghost) < 0)
     {
         printfPQExpBuffer(&conn->errorMessage, libpq_gettext("out of memory\n"));
         return STATUS_ERROR;
     }
-    sprintf(conn->sspitarget, "%s/%s", conn->krbsrvname, conn->pghost);

     /*
      * Indicate that we're in SSPI authentication mode to make sure that
diff --git a/src/port/asprintf.c b/src/port/asprintf.c
new file mode 100644
index 0000000..fafc03a
--- /dev/null
+++ b/src/port/asprintf.c
@@ -0,0 +1,111 @@
+/* src/port/asprintf.c */
+
+/* $NetBSD: asprintf.c,v 1.3 2012/07/02 16:02:53 joerg Exp $ */
+
+/*-
+ * Copyright (c) 2007 Joerg Sonnenberger <joerg@NetBSD.org>.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "c.h"
+
+#define HAVE_VA_COPY 1
+
+int
+asprintf(char **ret, const char *fmt, ...)
+{
+    va_list ap;
+    int retval;
+
+    va_start(ap, fmt);
+    retval = vasprintf(ret, fmt, ap);
+    va_end(ap);
+
+    return retval;
+}
+
+int
+vasprintf(char **ret, const char *fmt, va_list ap)
+{
+    char *buf, *new_buf;
+    size_t len;
+    int retval;
+    va_list ap2;
+
+    len = 128;
+    buf = malloc(len);
+    if (buf == NULL) {
+        *ret = NULL;
+        return -1;
+    }
+
+#if defined(HAVE_VA_COPY)
+    va_copy(ap2, ap);
+#define    my_va_end(ap2)    va_end(ap2)
+#elif defined(HAVE___BUILTIN_VA_COPY)
+    __builtin_va_copy(ap2, ap);
+#define    my_va_end(ap2)    __builtin_va_end(ap2)
+#else
+    ap2 = ap;
+#define    my_va_end(ap2)    do {} while (0)
+#endif
+    retval = vsnprintf(buf, len, fmt, ap);
+    if (retval < 0) {
+        free(buf);
+        *ret = NULL;
+        va_end(ap2);
+        return -1;
+    }
+
+    if (retval < len) {
+        new_buf = realloc(buf, retval + 1);
+        if (new_buf == NULL)
+            *ret = buf;
+        else
+            *ret = new_buf;
+        my_va_end(ap2);
+        return retval;
+    }
+
+    len = (size_t)retval + 1;
+    free(buf);
+    buf = malloc(len);
+    if (buf == NULL) {
+        *ret = NULL;
+        my_va_end(ap2);
+        return -1;
+    }
+    retval = vsnprintf(buf, len, fmt, ap2);
+    my_va_end(ap2);
+    if (retval != len - 1) {
+        free(buf);
+        *ret = NULL;
+        return -1;
+    }
+    *ret = buf;
+    return retval;
+}
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index f280779..25f3801 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -466,8 +466,7 @@ static void run_all_permutations_recurse(TestSpec * testspec, int nsteps,
 {
     char       *prefix;

-    prefix = malloc(strlen(step1->name) + strlen(step2->name) + 2);
-    sprintf(prefix, "%s %s", step1->name, step2->name);
+    pg_asprintf(&prefix, "%s %s", step1->name, step2->name);

     if (step1->errormsg)
     {
@@ -787,12 +786,9 @@ static void run_all_permutations_recurse(TestSpec * testspec, int nsteps,
                                                     PG_DIAG_MESSAGE_PRIMARY);

                     if (sev && msg)
-                    {
-                        step->errormsg = malloc(5 + strlen(sev) + strlen(msg));
-                        sprintf(step->errormsg, "%s:  %s", sev, msg);
-                    }
+                        pg_asprintf(&step->errormsg, "%s:  %s", sev, msg);
                     else
-                        step->errormsg = strdup(PQresultErrorMessage(res));
+                        step->errormsg = pg_strdup(PQresultErrorMessage(res));
                 }
                 break;
             default:
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index b632326..e51d088 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -654,9 +654,9 @@
 static void
 doputenv(const char *var, const char *val)
 {
-    char       *s = malloc(strlen(var) + strlen(val) + 2);
+    char       *s;

-    sprintf(s, "%s=%s", var, val);
+    pg_asprintf(&s, "%s=%s", var, val);
     putenv(s);
 }

@@ -671,16 +671,11 @@
     char       *newval;

     if (!oldval || !oldval[0])
-    {
         /* no previous value */
-        newval = malloc(strlen(pathname) + strlen(addval) + 2);
-        sprintf(newval, "%s=%s", pathname, addval);
-    }
+        pg_asprintf(&newval, "%s=%s", pathname, addval);
     else
-    {
-        newval = malloc(strlen(pathname) + strlen(addval) + strlen(oldval) + 3);
-        sprintf(newval, "%s=%s%c%s", pathname, addval, separator, oldval);
-    }
+        pg_asprintf(&newval, "%s=%s%c%s", pathname, addval, separator, oldval);
+
     putenv(newval);
 }

@@ -747,8 +742,7 @@

         if (!old_pgoptions)
             old_pgoptions = "";
-        new_pgoptions = malloc(strlen(old_pgoptions) + strlen(my_pgoptions) + 12);
-        sprintf(new_pgoptions, "PGOPTIONS=%s %s", old_pgoptions, my_pgoptions);
+        pg_asprintf(&new_pgoptions, "PGOPTIONS=%s %s", old_pgoptions, my_pgoptions);
         putenv(new_pgoptions);
     }

@@ -798,16 +792,13 @@
         /*
          * Adjust path variables to point into the temp-install tree
          */
-        tmp = malloc(strlen(temp_install) + 32 + strlen(bindir));
-        sprintf(tmp, "%s/install/%s", temp_install, bindir);
+        pg_asprintf(&tmp, "%s/install/%s", temp_install, bindir);
         bindir = tmp;

-        tmp = malloc(strlen(temp_install) + 32 + strlen(libdir));
-        sprintf(tmp, "%s/install/%s", temp_install, libdir);
+        pg_asprintf(&tmp, "%s/install/%s", temp_install, libdir);
         libdir = tmp;

-        tmp = malloc(strlen(temp_install) + 32 + strlen(datadir));
-        sprintf(tmp, "%s/install/%s", temp_install, datadir);
+        pg_asprintf(&tmp, "%s/install/%s", temp_install, datadir);
         datadir = tmp;

         /* psql will be installed into temp-install bindir */
@@ -961,9 +952,9 @@
          * "exec" the command too.    This saves two useless processes per
          * parallel test case.
          */
-        char       *cmdline2 = malloc(strlen(cmdline) + 6);
+        char       *cmdline2;

-        sprintf(cmdline2, "exec %s", cmdline);
+        pg_asprintf(&cmdline2, "exec %s", cmdline);
         execl(shellprog, shellprog, "-c", cmdline2, (char *) NULL);
         fprintf(stderr, _("%s: could not exec \"%s\": %s\n"),
                 progname, shellprog, strerror(errno));
@@ -1040,8 +1031,7 @@
         exit(2);
     }

-    cmdline2 = malloc(strlen(cmdline) + 8);
-    sprintf(cmdline2, "cmd /c %s", cmdline);
+    pg_asprintf(&cmdline2, "cmd /c %s", cmdline);

 #ifndef __CYGWIN__
     AddUserToTokenDacl(restrictedToken);
@@ -1862,8 +1852,7 @@
             }
         }

-        result = malloc(strlen(cwdbuf) + strlen(in) + 2);
-        sprintf(result, "%s/%s", cwdbuf, in);
+        pg_asprintf(&result, "%s/%s", cwdbuf, in);
     }

     canonicalize_path(result);
--
1.8.4.rc3

Peter Eisentraut wrote:

> The attached patch should speak for itself.

Yeah, it's a very nice cleanup.

> I have supplied a few variants:
> 
> - asprintf() is the standard function, supplied by libpgport if
> necessary.
> 
> - pg_asprintf() is asprintf() with automatic error handling (like
> pg_malloc(), etc.)
> 
> - psprintf() is the same idea but with palloc.

Looks good to me, except that pg_asprintf seems to be checking ret
instead of rc.

Is there a reason for the API discrepancy of pg_asprintf vs. psprintf?
I don't see that we use the integer return value anywhere.  Callers
interested in the return value can use asprintf directly (and you have
already inserted callers that do nonstandard things using direct
asprintf).

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, 2013-09-16 at 17:31 -0300, Alvaro Herrera wrote:
> Looks good to me, except that pg_asprintf seems to be checking ret
> instead of rc.

Ah, good catch!

> Is there a reason for the API discrepancy of pg_asprintf vs. psprintf?
> I don't see that we use the integer return value anywhere.  Callers
> interested in the return value can use asprintf directly (and you have
> already inserted callers that do nonstandard things using direct
> asprintf).

I wanted to keep pg_asprintf the same as asprintf.  I think there is
some value in that, but it's not supremely important.

On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote:

> 1. It seems that you have used strdup() on multiple places in the
> patch, e.g. in the below code snippet is it going to lead crash if
> newp->ident is NULL because of strdup() failure ?
> 
>         static EPlan *
>         find_plan(char *ident, EPlan **eplan, int *nplans)
>         {
>         ...
>              newp->ident = strdup(ident);
>         ...
>         }
> 
The previous code used unchecked malloc, so this doesn't change
anything.  It's only example code anyway.  (The use of malloc instead of
palloc is dubious anyway.)

> 
> 2. Can we rely on return value of asprintf() instead of recompute
> length as strlen(cmdbuf) ?
> 
>                   if (asprintf(&cmdbuf, "COMMENT ON LARGE OBJECT %u IS
>         '", loid) < 0)
>                        return fail_lo_xact("\\lo_import",
>         own_transaction);
>                   bufptr = cmdbuf + strlen(cmdbuf);
> 
Yes, good point.

> 3. There seems naming convention for functions like pfree (that seems
> similar to free()), pstrdup etc; but psprintf seems different than
> sprintf. Can't we use pg_asprintf instead in the patch, as it seems
> that both (psprintf and pg_asprintf) provide almost same
> functionality ?

pg_asprintf uses malloc, psprintf uses palloc, so they have to be
different functions.

The naming convention where

psomething = something with memory context management
pg_something = something with error checking

isn't very helpful, but it's what we have.  Better ideas welcome.

> 
> 5. It seems that in the previously implementation, error handling was
> not there, is it appropriate here that if there is issue in
> duplicating environment, quit the application ? i.e.
> 
> 
>         /*
>          * Handy subroutine for setting an environment variable "var"
>         to "val"
>          */
>         static void
>         doputenv(const char *var, const char *val)
>         {
>              char        *s;
>              pg_asprintf(&s, "%s=%s", var, val);
>              putenv(s);
>         }
> 
I think so, yes.

On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote:
> I did put some time review the patch, please see my findings below
> i.e.

Updated patch for this.

On 10/2/13 5:12 AM, Asif Naeem wrote:
> Neither git nor patch command apply the patch successfully. Can you
> please guide ?. Thanks.

Works for me.  Check that your email client isn't mangling line endings.

Peter Eisentraut escribió:
> On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote:
> > I did put some time review the patch, please see my findings below
> > i.e.
> 
> Updated patch for this.

Looks good to me.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote:

> 
> Looks like something like:
> 
> 
> #ifndef WIN32
> #define HAVE_VA_COPY 1
> #endif
> 
> 
> would need to be added to asprintf.c, but also some work needs to be
> done with mcxt.c as it uses va_copy unconditionally. Perhaps just
> defining a macro for va_copy would be better for windows. I was not
> quite sure the best header file for such a macro so I did not write a
> patch to fix it.

Does Windows not have va_copy?  What do they use instead?

On Tue, Oct 15, 2013 at 2:18 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote:
>
>>
>> Looks like something like:
>>
>>
>> #ifndef WIN32
>> #define HAVE_VA_COPY 1
>> #endif
>>
>>
>> would need to be added to asprintf.c, but also some work needs to be
>> done with mcxt.c as it uses va_copy unconditionally. Perhaps just
>> defining a macro for va_copy would be better for windows. I was not
>> quite sure the best header file for such a macro so I did not write a
>> patch to fix it.
>
> Does Windows not have va_copy?  What do they use instead?

No, Windows doesn't have va_copy, instead they use something like below:
#define va_copy(dest, src) (dest = src)

Please refer below link for details of porting va_copy() on Windows:
http://stackoverflow.com/questions/558223/va-copy-porting-to-visual-c

I could see that there is similar handling in code of vasprintf(),
such that if va_copy is not available then directly assign src to dst.

#if defined(HAVE_VA_COPY)
va_copy(ap2, ap);
#define my_va_end(ap2) va_end(ap2)
#elif defined(HAVE___BUILTIN_VA_COPY)
__builtin_va_copy(ap2, ap);
#define my_va_end(ap2) __builtin_va_end(ap2)
#else
ap2 = ap;
#define my_va_end(ap2) do {} while (0)
#endif

I think rather than having writing code like above at places where
va_copy is used, we can use something like:
#ifdef WIN32
#define va_copy(dest, src) (dest = src)
#endif

and define HAVE_VA_COPY to 1 for non-windows platform.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com