Re: [PATCH] Make jsonapi usable from libpq - Mailing list pgsql-hackers

I wrote:
> I spent some time looking for other undesirable symbol dependencies
> in libpq, and soon found a couple.  PGTHREAD_ERROR potentially calls
> abort(), which seems even worse than exit-on-OOM, although I don't
> think we've ever heard a report of that being hit.  Also,
> fe-print.c's handling of OOM isn't nice at all:
>                 fprintf(stderr, libpq_gettext("out of memory\n"));
>                 abort();
> Although fe-print.c is semi-deprecated, it still seems like it'd
> be a good idea to clean that up.

fe-print.c seems easy enough to clean up, as per attached.
Not real sure what to do about PGTHREAD_ERROR.

            regards, tom lane

diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c
index fc7d84844e..0831950b12 100644
--- a/src/interfaces/libpq/fe-print.c
+++ b/src/interfaces/libpq/fe-print.c
@@ -37,7 +37,7 @@
 #include "libpq-int.h"


-static void do_field(const PQprintOpt *po, const PGresult *res,
+static bool do_field(const PQprintOpt *po, const PGresult *res,
                      const int i, const int j, const int fs_len,
                      char **fields,
                      const int nFields, const char **fieldNames,
@@ -80,12 +80,12 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
         unsigned char *fieldNotNum = NULL;
         char       *border = NULL;
         char      **fields = NULL;
-        const char **fieldNames;
+        const char **fieldNames = NULL;
         int            fieldMaxLen = 0;
         int            numFieldName;
         int            fs_len = strlen(po->fieldSep);
         int            total_line_length = 0;
-        int            usePipe = 0;
+        bool        usePipe = false;
         char       *pagerenv;

 #if defined(ENABLE_THREAD_SAFETY) && !defined(WIN32)
@@ -111,17 +111,17 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
         if (!(fieldNames = (const char **) calloc(nFields, sizeof(char *))))
         {
             fprintf(stderr, libpq_gettext("out of memory\n"));
-            abort();
+            goto exit;
         }
         if (!(fieldNotNum = (unsigned char *) calloc(nFields, 1)))
         {
             fprintf(stderr, libpq_gettext("out of memory\n"));
-            abort();
+            goto exit;
         }
         if (!(fieldMax = (int *) calloc(nFields, sizeof(int))))
         {
             fprintf(stderr, libpq_gettext("out of memory\n"));
-            abort();
+            goto exit;
         }
         for (numFieldName = 0;
              po->fieldName && po->fieldName[numFieldName];
@@ -190,7 +190,7 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
                 fout = popen(pagerenv, "w");
                 if (fout)
                 {
-                    usePipe = 1;
+                    usePipe = true;
 #ifndef WIN32
 #ifdef ENABLE_THREAD_SAFETY
                     if (pq_block_sigpipe(&osigset, &sigpipe_pending) == 0)
@@ -207,10 +207,11 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)

         if (!po->expanded && (po->align || po->html3))
         {
-            if (!(fields = (char **) calloc(nFields * (nTups + 1), sizeof(char *))))
+            if (!(fields = (char **) calloc((size_t) nTups + 1,
+                                            nFields * sizeof(char *))))
             {
                 fprintf(stderr, libpq_gettext("out of memory\n"));
-                abort();
+                goto exit;
             }
         }
         else if (po->header && !po->html3)
@@ -264,9 +265,12 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
                     fprintf(fout, libpq_gettext("-- RECORD %d --\n"), i);
             }
             for (j = 0; j < nFields; j++)
-                do_field(po, res, i, j, fs_len, fields, nFields,
-                         fieldNames, fieldNotNum,
-                         fieldMax, fieldMaxLen, fout);
+            {
+                if (!do_field(po, res, i, j, fs_len, fields, nFields,
+                              fieldNames, fieldNotNum,
+                              fieldMax, fieldMaxLen, fout))
+                    goto exit;
+            }
             if (po->html3 && po->expanded)
                 fputs("</table>\n", fout);
         }
@@ -297,18 +301,34 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
             for (i = 0; i < nTups; i++)
                 output_row(fout, po, nFields, fields,
                            fieldNotNum, fieldMax, border, i);
-            free(fields);
-            if (border)
-                free(border);
         }
         if (po->header && !po->html3)
             fprintf(fout, "(%d row%s)\n\n", PQntuples(res),
                     (PQntuples(res) == 1) ? "" : "s");
         if (po->html3 && !po->expanded)
             fputs("</table>\n", fout);
-        free(fieldMax);
-        free(fieldNotNum);
-        free((void *) fieldNames);
+
+exit:
+        if (fieldMax)
+            free(fieldMax);
+        if (fieldNotNum)
+            free(fieldNotNum);
+        if (border)
+            free(border);
+        if (fields)
+        {
+            /* if calloc succeeded, this shouldn't overflow size_t */
+            size_t        numfields = ((size_t) nTups + 1) * (size_t) nFields;
+
+            while (numfields-- > 0)
+            {
+                if (fields[numfields])
+                    free(fields[numfields]);
+            }
+            free(fields);
+        }
+        if (fieldNames)
+            free((void *) fieldNames);
         if (usePipe)
         {
 #ifdef WIN32
@@ -329,7 +349,7 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
 }


-static void
+static bool
 do_field(const PQprintOpt *po, const PGresult *res,
          const int i, const int j, const int fs_len,
          char **fields,
@@ -397,7 +417,7 @@ do_field(const PQprintOpt *po, const PGresult *res,
             if (!(fields[i * nFields + j] = (char *) malloc(plen + 1)))
             {
                 fprintf(stderr, libpq_gettext("out of memory\n"));
-                abort();
+                return false;
             }
             strcpy(fields[i * nFields + j], pval);
         }
@@ -440,6 +460,7 @@ do_field(const PQprintOpt *po, const PGresult *res,
             }
         }
     }
+    return true;
 }


@@ -467,7 +488,7 @@ do_header(FILE *fout, const PQprintOpt *po, const int nFields, int *fieldMax,
         if (!border)
         {
             fprintf(stderr, libpq_gettext("out of memory\n"));
-            abort();
+            return NULL;
         }
         p = border;
         if (po->standard)
@@ -558,8 +579,6 @@ output_row(FILE *fout, const PQprintOpt *po, const int nFields, char **fields,
             if (po->standard || field_index + 1 < nFields)
                 fputs(po->fieldSep, fout);
         }
-        if (p)
-            free(p);
     }
     if (po->html3)
         fputs("</tr>", fout);
@@ -609,7 +628,7 @@ PQdisplayTuples(const PGresult *res,
         if (!fLength)
         {
             fprintf(stderr, libpq_gettext("out of memory\n"));
-            abort();
+            return;
         }

         for (j = 0; j < nFields; j++)
@@ -707,7 +726,7 @@ PQprintTuples(const PGresult *res,
             if (!tborder)
             {
                 fprintf(stderr, libpq_gettext("out of memory\n"));
-                abort();
+                return;
             }
             for (i = 0; i < width; i++)
                 tborder[i] = '-';