Fix more memory leaks in failure path in buildACLCommands.

tglsfdc · tglsfdc · commit 8b63f89498f2 · 2015-02-11T18:35:23.000-05:00
We already had one go at this issue in commit d73b7f9, but we failed to notice that buildACLCommands also leaked several PQExpBuffers along with a simply malloc'd string. This time let's try to make the fix a bit more future-proof by eliminating the separate exit path. It's still not exactly critical because pg_dump will curl up and die on failure; but since the amount of the potential leak is now several KB, it seems worth back-patching as far as 9.2 where the previous fix landed. Per Coverity, which evidently is smarter than clang's static analyzer.
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
@@ -531,6 +531,7 @@ buildACLCommands(const char *name, const char *subname,
 				 const char *prefix, int remoteVersion,
 				 PQExpBuffer sql)
 {
+	bool		ok = true;
 	char	  **aclitems;
 	int			naclitems;
 	int			i;
@@ -601,8 +602,8 @@ buildACLCommands(const char *name, const char *subname,
 		if (!parseAclItem(aclitems[i], type, name, subname, remoteVersion,
 						  grantee, grantor, privs, privswgo))
 		{
-			free(aclitems);
-			return false;
+			ok = false;
+			break;
 		}
 
 		if (grantor->len == 0 && owner)
@@ -709,7 +710,7 @@ buildACLCommands(const char *name, const char *subname,
 
 	free(aclitems);
 
-	return true;
+	return ok;
 }
 
 /*

Original file line number	Diff line number	Diff line change
`@@ -531,6 +531,7 @@ buildACLCommands(const char name, const char subname,`
`531`	`531`	`const char *prefix, int remoteVersion,`
`532`	`532`	`PQExpBuffer sql)`
`533`	`533`	`{`
	`534`	`+ bool ok = true;`
`534`	`535`	`char **aclitems;`
`535`	`536`	`int naclitems;`
`536`	`537`	`int i;`
`@@ -601,8 +602,8 @@ buildACLCommands(const char name, const char subname,`
`601`	`602`	`if (!parseAclItem(aclitems[i], type, name, subname, remoteVersion,`
`602`	`603`	`grantee, grantor, privs, privswgo))`
`603`	`604`	`{`
`604`		`- free(aclitems);`
`605`		`- return false;`
	`605`	`+ ok = false;`
	`606`	`+ break;`
`606`	`607`	`}`
`607`	`608`
`608`	`609`	`if (grantor->len == 0 && owner)`
`@@ -709,7 +710,7 @@ buildACLCommands(const char name, const char subname,`
`709`	`710`
`710`	`711`	`free(aclitems);`
`711`	`712`
`712`		`- return true;`
	`713`	`+ return ok;`
`713`	`714`	`}`
`714`	`715`
`715`	`716`	`/*`