Re: ParseTzFile doesn't FreeFile on error - Mailing list pgsql-hackers

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> The cause is ParseTzFile() returns leaving an open file descriptor
> unfreed in some error cases.
> This happens only in a special case when the errors are ignored, but
> in principle the file descriptor should be released before exiting the
> function.
> I'm not sure it's worth fixing but the attached fixes that.

I agree this is worth fixing, but adding all these gotos seems a bit
inelegant.  What do you think of the attached version?

BTW, my first thought about it was "what if one of the callees throws
elog(ERROR), eg palloc out-of-memory"?  But I think that's all right
since then we'll reach transaction abort cleanup, which won't whine
about open files.  The problem is limited to the case where no error
gets thrown.

            regards, tom lane

diff --git a/src/backend/utils/misc/tzparser.c b/src/backend/utils/misc/tzparser.c
index a69cb2d268..8f2c95f055 100644
--- a/src/backend/utils/misc/tzparser.c
+++ b/src/backend/utils/misc/tzparser.c
@@ -364,7 +364,8 @@ ParseTzFile(const char *filename, int depth,
             {
                 GUC_check_errmsg("could not read time zone file \"%s\": %m",
                                  filename);
-                return -1;
+                n = -1;
+                break;
             }
             /* else we're at EOF after all */
             break;
@@ -374,7 +375,8 @@ ParseTzFile(const char *filename, int depth,
             /* the line is too long for tzbuf */
             GUC_check_errmsg("line is too long in time zone file \"%s\", line %d",
                              filename, lineno);
-            return -1;
+            n = -1;
+            break;
         }

         /* skip over whitespace */
@@ -397,12 +399,13 @@ ParseTzFile(const char *filename, int depth,
             {
                 GUC_check_errmsg("@INCLUDE without file name in time zone file \"%s\", line %d",
                                  filename, lineno);
-                return -1;
+                n = -1;
+                break;
             }
             n = ParseTzFile(includeFile, depth + 1,
                             base, arraysize, n);
             if (n < 0)
-                return -1;
+                break;
             continue;
         }

@@ -413,12 +416,18 @@ ParseTzFile(const char *filename, int depth,
         }

         if (!splitTzLine(filename, lineno, line, &tzentry))
-            return -1;
+        {
+            n = -1;
+            break;
+        }
         if (!validateTzEntry(&tzentry))
-            return -1;
+        {
+            n = -1;
+            break;
+        }
         n = addToArray(base, arraysize, n, &tzentry, override);
         if (n < 0)
-            return -1;
+            break;
     }

     FreeFile(tzFile);