[Patch] timezone/zic.c: Fix file handle leak in dolink()

Started by zengman15 days ago5 messages
#1zengman
zengman@halodbtech.com
1 attachment(s)

Hi all,

I noticed a small issue and made a tiny patch to fix it — it addresses a file handle leak in the dolink() function of src/timezone/zic.c.

--
Regards,
Man Zeng
www.openhalo.org

Attachments:

zic_file_leak_fix.diffapplication/octet-stream; charset=gb18030; name=zic_file_leak_fix.diffDownload
diff --git a/src/timezone/zic.c b/src/timezone/zic.c
index 8dcc7b337a7..bee94fa08f1 100644
--- a/src/timezone/zic.c
+++ b/src/timezone/zic.c
@@ -1095,6 +1095,7 @@ dolink(char const *target, char const *linkname, bool staysymlink)
 			{
 				char const *e = strerror(errno);
 
+				fclose(fp);
 				fprintf(stderr, _("%s: Can't create %s/%s: %s\n"),
 						progname, directory, linkname, e);
 				exit(EXIT_FAILURE);
#2zengman
zengman@halodbtech.com
In reply to: zengman (#1)
1 attachment(s)
Re: [Patch] timezone/zic.c: Fix file handle leak in dolink()

Furthermore, I noticed that some of the test code in `contrib/pg_trgm/trgm_regexp.c` does not check the return value of the `fopen()` call.
I'm not sure if it's necessary to make the modification.

--
Regards,
Man Zeng
www.openhalo.org

Attachments:

pg_trgm-add-fopen-null-check.diffapplication/octet-stream; charset=ISO-8859-1; name=pg_trgm-add-fopen-null-check.diffDownload
diff --git a/contrib/pg_trgm/trgm_regexp.c b/contrib/pg_trgm/trgm_regexp.c
index 1a76794c422..5567a7339c0 100644
--- a/contrib/pg_trgm/trgm_regexp.c
+++ b/contrib/pg_trgm/trgm_regexp.c
@@ -2195,6 +2195,8 @@ printSourceNFA(regex_t *regex, TrgmColorInfo *colors, int ncolors)
 		/* dot -Tpng -o /tmp/source.png < /tmp/source.gv */
 		FILE	   *fp = fopen("/tmp/source.gv", "w");
 
+		if (!fp)
+			elog(ERROR, "could not open file \"/tmp/source.gv\": %m");
 		fprintf(fp, "%s", buf.data);
 		fclose(fp);
 	}
@@ -2257,6 +2259,8 @@ printTrgmNFA(TrgmNFA *trgmNFA)
 		/* dot -Tpng -o /tmp/transformed.png < /tmp/transformed.gv */
 		FILE	   *fp = fopen("/tmp/transformed.gv", "w");
 
+		if (!fp)
+			elog(ERROR, "could not open file \"/tmp/transformed.gv\": %m");
 		fprintf(fp, "%s", buf.data);
 		fclose(fp);
 	}
@@ -2348,6 +2352,8 @@ printTrgmPackedGraph(TrgmPackedGraph *packedGraph, TRGM *trigrams)
 		/* dot -Tpng -o /tmp/packed.png < /tmp/packed.gv */
 		FILE	   *fp = fopen("/tmp/packed.gv", "w");
 
+		if (!fp)
+			elog(ERROR, "could not open file \"/tmp/packed.gv\": %m");
 		fprintf(fp, "%s", buf.data);
 		fclose(fp);
 	}
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: zengman (#1)
Re: [Patch] timezone/zic.c: Fix file handle leak in dolink()

"=?gb18030?B?emVuZ21hbg==?=" <zengman@halodbtech.com> writes:

I noticed a small issue and made a tiny patch to fix it ¡ª it addresses a file handle leak in the dolink() function of src/timezone/zic.c.

Huh? The program is going to exit() two lines further down.
What's the point of closing here?

Even if there were a plausible argument for issuing close(),
I wouldn't care to diverge from upstream zic. If you like
you can go try to persuade the tzdb crew that this is a
useful change.

regards, tom lane

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: zengman (#2)
Re: [Patch] timezone/zic.c: Fix file handle leak in dolink()

"=?ISO-8859-1?B?emVuZ21hbg==?=" <zengman@halodbtech.com> writes:

Furthermore, I noticed that some of the test code in `contrib/pg_trgm/trgm_regexp.c` does not check the return value of the `fopen()` call.
I'm not sure if it's necessary to make the modification.

Probably not worth the trouble. This is barely even debug-grade
code, seeing that there's no provision to change the target file
name. It's clearly not meant for use in unfriendly environments.

regards, tom lane

#5zengman
zengman@halodbtech.com
In reply to: Tom Lane (#4)
Re: [Patch] timezone/zic.c: Fix file handle leak in dolink()

Got it, thank you for your guidance.

--
Regards,
Man Zeng
www.openhalo.org