Re: [HACKERS] Small patch: fix double variable initializations in policy.c

[Tom Lane]

Aleksander Alekseev  writes:
> I'm quite sure that there is no need to initialize these variables
> twice. First patch fixes this. Also I discovered that policy.c is not
> properly pgindent'ed. Second patch fixes this too.

Applied the first patch --- though I chose to do it the other way
(keep the declaration and assignment separate) because as you had it,
pgindent would've inserted an ugly/confusing blank line within the
declarations stanza of the block.

With the end of the devel cycle fast approaching, we might as well leave
it to the regular pgindent run to clean up the other issues.  People tend
to complain if you invalidate pending patches at this time of year ;-)

Thanks for catching this!

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Small patch: fix double variable initializations in policy.c

[Aleksander Alekseev]

Hello

I discovered a pretty weird code.

policy.c:1083

```
char   *qual_value;
ParseState *qual_pstate = make_parsestate(NULL);

/* parsestate is built just to build the range table */
qual_pstate = make_parsestate(NULL);
```

policy.c:1125

```
char   *with_check_value;
ParseState *with_check_pstate = make_parsestate(NULL);

/* parsestate is built just to build the range table */
with_check_pstate = make_parsestate(NULL);
```

I'm quite sure that there is no need to initialize these variables
twice. First patch fixes this. Also I discovered that policy.c is not
properly pgindent'ed. Second patch fixes this too.

Naturally I checked that after applying these patches PostgreSQL still
compiles and pass all regression tests.

-- 
Best regards,
Aleksander Alekseev
http://eax.me/
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index bb735ac..dfc6f00 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -1081,10 +1081,8 @@ AlterPolicy(AlterPolicyStmt *stmt)
 		if (!attr_isnull)
 		{
 			char	   *qual_value;
-			ParseState *qual_pstate = make_parsestate(NULL);
-
 			/* parsestate is built just to build the range table */
-			qual_pstate = make_parsestate(NULL);
+			ParseState *qual_pstate = make_parsestate(NULL);
 
 			qual_value = TextDatumGetCString(value_datum);
 			qual = stringToNode(qual_value);
@@ -1122,10 +1120,8 @@ AlterPolicy(AlterPolicyStmt *stmt)
 		if (!attr_isnull)
 		{
 			char	   *with_check_value;
-			ParseState *with_check_pstate = make_parsestate(NULL);
-
 			/* parsestate is built just to build the range table */
-			with_check_pstate = make_parsestate(NULL);
+			ParseState *with_check_pstate = make_parsestate(NULL);
 
 			with_check_value = TextDatumGetCString(value_datum);
 			with_check_qual = stringToNode(with_check_value);
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index dfc6f00..22fa344 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -496,7 +496,7 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
 
 	/* Must own relation. */
 	if (pg_class_ownercheck(relid, GetUserId()))
-		noperm = false; /* user is allowed to modify this policy */
+		noperm = false;			/* user is allowed to modify this policy */
 	else
 		ereport(WARNING,
 (errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
@@ -511,15 +511,16 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
 	 */
 	if (!noperm && num_roles > 0)
 	{
-		int			i, j;
+		int			i,
+	j;
 		Oid		   *roles = (Oid *) ARR_DATA_PTR(policy_roles);
 		Datum	   *role_oids;
 		char	   *qual_value;
 		Node	   *qual_expr;
-		List   *qual_parse_rtable = NIL;
+		List	   *qual_parse_rtable = NIL;
 		char	   *with_check_value;
 		Node	   *with_check_qual;
-		List   *with_check_parse_rtable = NIL;
+		List	   *with_check_parse_rtable = NIL;
 		Datum		values[Natts_pg_policy];
 		bool		isnull[Natts_pg_policy];
 		bool		replaces[Natts_pg_policy];
@@ -536,15 +537,14 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
 
 		/*
 		 * All of the dependencies will be removed from the policy and then
-		 * re-added.  In order to get them correct, we need to extract out
-		 * the expressions in the policy and construct a parsestate just
-		 * enough to build the range table(s) to then pass to
-		 * recordDependencyOnExpr().
+		 * re-added.  In order to get them correct, we need to extract out the
+		 * expressions in the policy and construct a parsestate just enough to
+		 * build the range table(s) to then pass to recordDependencyOnExpr().
 		 */
 
 		/* Get policy qual, to update dependencies */
 		value_datum = heap_getattr(tuple, Anum_pg_policy_polqual,
-   RelationGetDescr(pg_policy_rel), _isnull);
+			  RelationGetDescr(pg_policy_rel), _isnull);
 		if (!attr_isnull)
 		{
 			ParseState *qual_pstate;
@@ -566,7 +566,7 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
 
 		/* Get WITH CHECK qual, to update dependencies */
 		value_datum = heap_getattr(tuple, Anum_pg_policy_polwithcheck,
-   RelationGetDescr(pg_policy_rel), _isnull);
+			  RelationGetDescr(pg_policy_rel), _isnull);
 		if (!attr_isnull)
 		{
 			ParseState *with_check_pstate;
@@ -665,7 +665,7 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
 
 	heap_close(pg_policy_rel, RowExclusiveLock);
 
-	return(noperm || num_roles > 0);
+	return (noperm || num_roles > 0);
 }
 
 /*
@@ -996,8 +996,8 @@ AlterPolicy(AlterPolicyStmt *stmt)
 
 	/* Get policy command */
 	polcmd_datum = heap_getattr(policy_tuple, Anum_pg_policy_polcmd,
-			 RelationGetDescr(pg_policy_rel),
-			 _isnull);
+RelationGetDescr(pg_policy_rel),
+_isnull);
 	Assert(!polcmd_isnull);
 	polcmd = DatumGetChar(polcmd_datum);
 
@@ -1029,15 +1029,15 @@ AlterPolicy(AlterPolicyStmt *stmt)
 	}
 	else
 	{
-		Oid*roles;
+		Oid		   *roles;
 		Datum		roles_datum;
 		bool