Mop up some no-longer-necessary hacks around printf %.*s format.

Commit 54cd4f045 added some kluges to work around an old glibc bug,
namely that %.*s could misbehave if glibc thought any characters in
the supplied string were incorrectly encoded.  Now that we use our
own snprintf.c implementation, we need not worry about that bug (even
if it still exists in the wild).  Revert a couple of particularly
ugly hacks, and remove or improve assorted comments.

Note that there can still be encoding-related hazards here: blindly
clipping at a fixed length risks producing wrongly-encoded output
if the clip splits a multibyte character.  However, code that's
doing correct multibyte-aware clipping doesn't really need a comment
about that, while code that isn't needs an explanation why not,
rather than a red-herring comment about an obsolete bug.

Discussion: https://postgr.es/m/279428.1593373684@sss.pgh.pa.us

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 6b1fd6d4cca6683d92e4163792759392d5e56d50..3e199bdfd0c6520a455f821df8acfedc0df76ede 100644 (file)
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2303,11 +2303,7 @@ CopyFromErrorCallback(void *arg)
 /*
  * Make sure we don't print an unreasonable amount of COPY data in a message.
  *
- * It would seem a lot easier to just use the sprintf "precision" limit to
- * truncate the string.  However, some versions of glibc have a bug/misfeature
- * that vsnprintf will always fail (return -1) if it is asked to truncate
- * a string that contains invalid byte sequences for the current encoding.
- * So, do our own truncation.  We return a pstrdup'd copy of the input.
+ * Returns a pstrdup'd copy of the input.
  */
 static char *
 limit_printout_length(const char *str)

diff --git a/src/backend/parser/scansup.c b/src/backend/parser/scansup.c
index 18169ec4f4cc0e6ba69d9306da96f6e7c72750a7..cac70d5df7afca40672312d77081027c8f457c20 100644 (file)
--- a/src/backend/parser/scansup.c
+++ b/src/backend/parser/scansup.c
@@ -189,20 +189,10 @@ truncate_identifier(char *ident, int len, bool warn)
    {
        len = pg_mbcliplen(ident, len, NAMEDATALEN - 1);
        if (warn)
-       {
-           /*
-            * We avoid using %.*s here because it can misbehave if the data
-            * is not valid in what libc thinks is the prevailing encoding.
-            */
-           char        buf[NAMEDATALEN];
-
-           memcpy(buf, ident, len);
-           buf[len] = '\0';
            ereport(NOTICE,
                    (errcode(ERRCODE_NAME_TOO_LONG),
-                    errmsg("identifier \"%s\" will be truncated to \"%s\"",
-                           ident, buf)));
-       }
+                    errmsg("identifier \"%s\" will be truncated to \"%.*s\"",
+                           ident, len, ident)));
        ident[len] = '\0';
    }
 }

diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index 48e55e141a4115945865e8002b38c77403e9eeee..fda35abc74179a416cc36b0a4a938eff84dcd978 100644 (file)
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -324,12 +324,6 @@ TParserInit(char *str, int len)
    prs->state->state = TPS_Base;
 
 #ifdef WPARSER_TRACE
-
-   /*
-    * Use of %.*s here is a bit risky since it can misbehave if the data is
-    * not in what libc thinks is the prevailing encoding.  However, since
-    * this is just a debugging aid, we choose to live with that.
-    */
    fprintf(stderr, "parsing \"%.*s\"\n", len, str);
 #endif
 
@@ -366,7 +360,6 @@ TParserCopyInit(const TParser *orig)
    prs->state->state = TPS_Base;
 
 #ifdef WPARSER_TRACE
-   /* See note above about %.*s */
    fprintf(stderr, "parsing copy of \"%.*s\"\n", prs->lenstr, prs->str);
 #endif
 

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 0b6dfb248cbcb74431766a8ecd2d343f18535a39..dec2fad82a685030f0df9c2fb8a66576ae49c000 100644 (file)
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -4013,7 +4013,8 @@ EncodeDateTime(struct pg_tm *tm, fsec_t fsec, bool print_tz, int tz, const char
 
            /*
             * Note: the uses of %.*s in this function would be risky if the
-            * timezone names ever contain non-ASCII characters.  However, all
+            * timezone names ever contain non-ASCII characters, since we are
+            * not being careful to do encoding-aware clipping.  However, all
             * TZ abbreviations in the IANA database are plain ASCII.
             */
            if (print_tz)

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 076c3c019ff425852dfdcb9ce19cf8f0602ddf87..2cbcb4b85e3b2b2d5a8a87db5207892357b2ce0e 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -3554,11 +3554,6 @@ set_rtable_names(deparse_namespace *dpns, List *parent_namespaces,
                    hentry->counter++;
                    for (;;)
                    {
-                       /*
-                        * We avoid using %.*s here because it can misbehave
-                        * if the data is not valid in what libc thinks is the
-                        * prevailing encoding.
-                        */
                        memcpy(modname, refname, refnamelen);
                        sprintf(modname + refnamelen, "_%d", hentry->counter);
                        if (strlen(modname) < NAMEDATALEN)
@@ -4438,11 +4433,6 @@ make_colname_unique(char *colname, deparse_namespace *dpns,
            i++;
            for (;;)
            {
-               /*
-                * We avoid using %.*s here because it can misbehave if the
-                * data is not valid in what libc thinks is the prevailing
-                * encoding.
-                */
                memcpy(modname, colname, colnamelen);
                sprintf(modname + colnamelen, "_%d", i);
                if (strlen(modname) < NAMEDATALEN)

diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index 66a50f183f52597a1efb468e22927e801a7c02ee..508f537c0c7a681ec614a44afd8e7c553e828d16 100644 (file)
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -305,20 +305,6 @@ format_numeric_locale(const char *my_str)
 }
 
 
-/*
- * fputnbytes: print exactly N bytes to a file
- *
- * We avoid using %.*s here because it can misbehave if the data
- * is not valid in what libc thinks is the prevailing encoding.
- */
-static void
-fputnbytes(FILE *f, const char *str, size_t n)
-{
-   while (n-- > 0)
-       fputc(*str++, f);
-}
-
-
 static void
 print_separator(struct separator sep, FILE *fout)
 {
@@ -1042,16 +1028,14 @@ print_aligned_text(const printTableContent *cont, FILE *fout, bool is_pager)
                    {
                        /* spaces first */
                        fprintf(fout, "%*s", width_wrap[j] - chars_to_output, "");
-                       fputnbytes(fout,
-                                  (char *) (this_line->ptr + bytes_output[j]),
-                                  bytes_to_output);
+                       fwrite((char *) (this_line->ptr + bytes_output[j]),
+                              1, bytes_to_output, fout);
                    }
                    else        /* Left aligned cell */
                    {
                        /* spaces second */
-                       fputnbytes(fout,
-                                  (char *) (this_line->ptr + bytes_output[j]),
-                                  bytes_to_output);
+                       fwrite((char *) (this_line->ptr + bytes_output[j]),
+                              1, bytes_to_output, fout);
                    }
 
                    bytes_output[j] += bytes_to_output;
@@ -1637,8 +1621,8 @@ print_aligned_vertical(const printTableContent *cont,
                 */
                bytes_to_output = strlen_max_width(dlineptr[dline].ptr + offset,
                                                   &target_width, encoding);
-               fputnbytes(fout, (char *) (dlineptr[dline].ptr + offset),
-                          bytes_to_output);
+               fwrite((char *) (dlineptr[dline].ptr + offset),
+                      1, bytes_to_output, fout);
 
                chars_to_output -= target_width;
                offset += bytes_to_output;

diff --git a/src/interfaces/ecpg/ecpglib/error.c b/src/interfaces/ecpg/ecpglib/error.c
index a4e3c0d01f8c6f0e022b77db95453dbd0bddc731..cd6c6a6819bfa5401e1fe438c7d1a07ae4621fdf 100644 (file)
--- a/src/interfaces/ecpg/ecpglib/error.c
+++ b/src/interfaces/ecpg/ecpglib/error.c
@@ -270,7 +270,6 @@ ecpg_raise_backend(int line, PGresult *result, PGconn *conn, int compat)
    else
        sqlca->sqlcode = ECPG_PGSQL;
 
-   /* %.*s is safe here as long as sqlstate is all-ASCII */
    ecpg_log("raising sqlstate %.*s (sqlcode %ld): %s\n",
             (int) sizeof(sqlca->sqlstate), sqlca->sqlstate, sqlca->sqlcode, sqlca->sqlerrm.sqlerrmc);
 

diff --git a/src/interfaces/ecpg/pgtypeslib/dt_common.c b/src/interfaces/ecpg/pgtypeslib/dt_common.c
index 81bd7aa526f8420efb6bf71dad1d089e194f08b6..14cdf2d428b5d1eec1eef9666bc6b2a6369701d3 100644 (file)
--- a/src/interfaces/ecpg/pgtypeslib/dt_common.c
+++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c
@@ -826,7 +826,8 @@ EncodeDateTime(struct tm *tm, fsec_t fsec, bool print_tz, int tz, const char *tz
 
            /*
             * Note: the uses of %.*s in this function would be risky if the
-            * timezone names ever contain non-ASCII characters.  However, all
+            * timezone names ever contain non-ASCII characters, since we are
+            * not being careful to do encoding-aware clipping.  However, all
             * TZ abbreviations in the IANA database are plain ASCII.
             */
 

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 9273984727a25fb2defdd1e0de332a5ed977561e..ff840b7730d81d765f9a457fad685fc8f660c998 100644 (file)
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -68,19 +68,6 @@ PQlibVersion(void)
    return PG_VERSION_NUM;
 }
 
-/*
- * fputnbytes: print exactly N bytes to a file
- *
- * We avoid using %.*s here because it can misbehave if the data
- * is not valid in what libc thinks is the prevailing encoding.
- */
-static void
-fputnbytes(FILE *f, const char *str, size_t n)
-{
-   while (n-- > 0)
-       fputc(*str++, f);
-}
-
 
 /*
  * pqGetc: get 1 character from the connection
@@ -204,7 +191,7 @@ pqGetnchar(char *s, size_t len, PGconn *conn)
    if (conn->Pfdebug)
    {
        fprintf(conn->Pfdebug, "From backend (%lu)> ", (unsigned long) len);
-       fputnbytes(conn->Pfdebug, s, len);
+       fwrite(s, 1, len, conn->Pfdebug);
        fprintf(conn->Pfdebug, "\n");
    }
 
@@ -228,7 +215,7 @@ pqSkipnchar(size_t len, PGconn *conn)
    if (conn->Pfdebug)
    {
        fprintf(conn->Pfdebug, "From backend (%lu)> ", (unsigned long) len);
-       fputnbytes(conn->Pfdebug, conn->inBuffer + conn->inCursor, len);
+       fwrite(conn->inBuffer + conn->inCursor, 1, len, conn->Pfdebug);
        fprintf(conn->Pfdebug, "\n");
    }
 
@@ -250,7 +237,7 @@ pqPutnchar(const char *s, size_t len, PGconn *conn)
    if (conn->Pfdebug)
    {
        fprintf(conn->Pfdebug, "To backend> ");
-       fputnbytes(conn->Pfdebug, s, len);
+       fwrite(s, 1, len, conn->Pfdebug);
        fprintf(conn->Pfdebug, "\n");
    }