Make acl-related functions safe for TOAST. Mark pg_class.relacl as

compressible but not externally storable (since we're not sure about
whether creating a toast relation for pg_class would work).

3 files changed, 126 insertions, 135 deletions

diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 482bb924dcb..bab11857303 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/heap/tuptoaster.c,v 1.9 2000/07/22 11:18:46 wieck Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/heap/tuptoaster.c,v 1.10 2000/07/31 22:39:17 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -273,7 +273,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
 
 			/* ----------
 			 * If the old value is an external stored one, check if it
-			 * has changed so we have to detele it later.
+			 * has changed so we have to delete it later.
 			 * ----------
 			 */
 			if (!old_isnull && att[i]->attlen == -1 && 
@@ -336,17 +336,16 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
 		if (att[i]->attlen == -1)
 		{
 			/* ----------
-			 * If the tables attribute say's PLAIN allways, we
-			 * do so below.
+			 * If the table's attribute says PLAIN always, force it so.
 			 * ----------
 			 */
 			if (att[i]->attstorage == 'p')
 				toast_action[i] = 'p';
 
 			/* ----------
-			 * We're running for UPDATE, so any TOASTed value we find
-			 * still in the tuple must be someone elses we cannot reuse.
-			 * Expand it to plain and eventually toast it again below.
+			 * We took care of UPDATE above, so any TOASTed value we find
+			 * still in the tuple must be someone else's we cannot reuse.
+			 * Expand it to plain (and, probably, toast it again below).
 			 * ----------
 			 */
 			if (VARATT_IS_EXTENDED(DatumGetPointer(toast_values[i])))
@@ -367,7 +366,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup)
 		else
 		{
 			/* ----------
-			 * Not a variable size attribute, plain storage allways
+			 * Not a variable size attribute, plain storage always
 			 * ----------
 			 */
 			toast_action[i] = 'p';
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index c342b067ff4..6be489a0c74 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.38 2000/04/12 17:14:55 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.39 2000/07/31 22:39:13 tgl Exp $
  *
  * NOTES
  *	  See acl.h.
@@ -33,7 +33,8 @@
 #include "utils/acl.h"
 #include "utils/syscache.h"
 
-static int32 aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode);
+static int32 aclcheck(char *relname, Acl *acl, AclId id,
+					  AclIdType idtype, AclMode mode);
 
 /*
  * Enable use of user relations in place of real system catalogs.
@@ -68,14 +69,16 @@ char	   *aclcheck_error_strings[] = {
 static
 dumpacl(Acl *acl)
 {
-	unsigned	i;
+	int			i;
 	AclItem    *aip;
 
 	elog(DEBUG, "acl size = %d, # acls = %d",
 		 ACL_SIZE(acl), ACL_NUM(acl));
-	aip = (AclItem *) ACL_DAT(acl);
+	aip = ACL_DAT(acl);
 	for (i = 0; i < ACL_NUM(acl); ++i)
-		elog(DEBUG, "	acl[%d]: %s", i, aclitemout(aip + i));
+		elog(DEBUG, "	acl[%d]: %s", i,
+			 DatumGetCString(DirectFunctionCall1(aclitemout,
+												 PointerGetDatum(aip + i))));
 }
 
 #endif
@@ -89,7 +92,7 @@ ChangeAcl(char *relname,
 		  unsigned modechg)
 {
 	unsigned	i;
-	Acl		   *old_acl = (Acl *) NULL,
+	Acl		   *old_acl,
 			   *new_acl;
 	Relation	relation;
 	HeapTuple	tuple;
@@ -97,14 +100,12 @@ ChangeAcl(char *relname,
 	char		nulls[Natts_pg_class];
 	char		replaces[Natts_pg_class];
 	Relation	idescs[Num_pg_class_indices];
-	int			free_old_acl = 0;
+	bool		isNull;
+	bool		free_old_acl = false;
 
 	/*
 	 * Find the pg_class tuple matching 'relname' and extract the ACL. If
 	 * there's no ACL, create a default using the pg_class.relowner field.
-	 *
-	 * We can't use the syscache here, since we need to do a heap_update on
-	 * the tuple we find.
 	 */
 	relation = heap_openr(RelationRelationName, RowExclusiveLock);
 	tuple = SearchSysCacheTuple(RELNAME,
@@ -117,25 +118,37 @@ ChangeAcl(char *relname,
 			 relname);
 	}
 
-	if (!heap_attisnull(tuple, Anum_pg_class_relacl))
-		old_acl = (Acl *) heap_getattr(tuple,
-									   Anum_pg_class_relacl,
-									   RelationGetDescr(relation),
-									   (bool *) NULL);
-	if (!old_acl || ACL_NUM(old_acl) < 1)
+	old_acl = (Acl *) heap_getattr(tuple,
+								   Anum_pg_class_relacl,
+								   RelationGetDescr(relation),
+								   &isNull);
+	if (isNull)
 	{
 #ifdef ACLDEBUG_TRACE
 		elog(DEBUG, "ChangeAcl: using default ACL");
 #endif
-/*		old_acl = acldefault(((Form_pg_class) GETSTRUCT(tuple))->relowner); */
 		old_acl = acldefault(relname);
-		free_old_acl = 1;
+		free_old_acl = true;
+	}
+
+	/* Need to detoast the old ACL for modification */
+	old_acl = DatumGetAclP(PointerGetDatum(old_acl));
+
+	if (ACL_NUM(old_acl) < 1)
+	{
+#ifdef ACLDEBUG_TRACE
+		elog(DEBUG, "ChangeAcl: old ACL has zero length");
+#endif
+		old_acl = acldefault(relname);
+		free_old_acl = true;
 	}
 
 #ifdef ACLDEBUG_TRACE
 	dumpacl(old_acl);
 #endif
+
 	new_acl = aclinsert3(old_acl, mod_aip, modechg);
+
 #ifdef ACLDEBUG_TRACE
 	dumpacl(new_acl);
 #endif
@@ -148,7 +161,7 @@ ChangeAcl(char *relname,
 										 * anyway */
 	}
 	replaces[Anum_pg_class_relacl - 1] = 'r';
-	values[Anum_pg_class_relacl - 1] = (Datum) new_acl;
+	values[Anum_pg_class_relacl - 1] = PointerGetDatum(new_acl);
 	tuple = heap_modifytuple(tuple, relation, values, nulls, replaces);
 
 	heap_update(relation, &tuple->t_self, tuple, NULL);
@@ -193,20 +206,20 @@ get_groname(AclId grosysid)
 	if (HeapTupleIsValid(tuple))
 		name = NameStr(((Form_pg_group) GETSTRUCT(tuple))->groname);
 	else
-		elog(NOTICE, "get_groname: group %d not found", grosysid);
+		elog(NOTICE, "get_groname: group %u not found", grosysid);
 	return name;
 }
 
-static int32
+static bool
 in_group(AclId uid, AclId gid)
 {
 	Relation	relation;
 	HeapTuple	tuple;
 	Acl		   *tmp;
-	unsigned	i,
+	int			i,
 				num;
 	AclId	   *aidp;
-	int32		found = 0;
+	bool		found = false;
 
 	relation = heap_openr(GroupRelationName, RowExclusiveLock);
 	tuple = SearchSysCacheTuple(GROSYSID,
@@ -219,13 +232,15 @@ in_group(AclId uid, AclId gid)
 									  Anum_pg_group_grolist,
 									  RelationGetDescr(relation),
 									  (bool *) NULL);
+		/* be sure the IdList is not toasted */
+		tmp = DatumGetIdListP(PointerGetDatum(tmp));
 		/* XXX make me a function */
 		num = IDLIST_NUM(tmp);
 		aidp = IDLIST_DAT(tmp);
 		for (i = 0; i < num; ++i)
 			if (aidp[i] == uid)
 			{
-				found = 1;
+				found = true;
 				break;
 			}
 	}
@@ -344,8 +359,7 @@ pg_aclcheck(char *relname, char *usename, AclMode mode)
 {
 	HeapTuple	tuple;
 	AclId		id;
-	Acl		   *acl = (Acl *) NULL,
-			   *tmp;
+	Acl		   *acl = (Acl *) NULL;
 	int32		result;
 	Relation	relation;
 
@@ -396,12 +410,11 @@ pg_aclcheck(char *relname, char *usename, AclMode mode)
 	}
 	if (!heap_attisnull(tuple, Anum_pg_class_relacl))
 	{
-		tmp = (Acl *) heap_getattr(tuple,
-								   Anum_pg_class_relacl,
-								   RelationGetDescr(relation),
-								   (bool *) NULL);
-		acl = makeacl(ACL_NUM(tmp));
-		memmove((char *) acl, (char *) tmp, ACL_SIZE(tmp));
+		/* get a detoasted copy of the ACL */
+		acl = DatumGetAclPCopy(heap_getattr(tuple,
+											Anum_pg_class_relacl,
+											RelationGetDescr(relation),
+											(bool *) NULL));
 	}
 	else
 	{
@@ -410,13 +423,10 @@ pg_aclcheck(char *relname, char *usename, AclMode mode)
 		 * if the acl is null, by default the owner can do whatever he
 		 * wants to with it
 		 */
-		int4		ownerId;
+		AclId		ownerId;
 
-		ownerId = (int4) heap_getattr(tuple,
-									  Anum_pg_class_relowner,
-									  RelationGetDescr(relation),
-									  (bool *) NULL);
-		acl = aclownerdefault(relname, (AclId) ownerId);
+		ownerId = ((Form_pg_class) GETSTRUCT(tuple))->relowner;
+		acl = aclownerdefault(relname, ownerId);
 	}
 	heap_close(relation, RowExclusiveLock);
 #else
@@ -427,12 +437,11 @@ pg_aclcheck(char *relname, char *usename, AclMode mode)
 	if (HeapTupleIsValid(tuple) &&
 		!heap_attisnull(tuple, Anum_pg_class_relacl))
 	{
-		tmp = (Acl *) heap_getattr(tuple,
-								   Anum_pg_class_relacl,
-								   RelationGetDescr(relation),
-								   (bool *) NULL);
-		acl = makeacl(ACL_NUM(tmp));
-		memmove((char *) acl, (char *) tmp, ACL_SIZE(tmp));
+		/* get a detoasted copy of the ACL */
+		acl = DatumGetAclPCopy(heap_getattr(tuple,
+											Anum_pg_class_relacl,
+											RelationGetDescr(relation),
+											(bool *) NULL));
 	}
 	heap_close(relation, RowExclusiveLock);
 #endif
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 4cbaba9b12f..c4f1228dbcf 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v 1.47 2000/06/14 18:17:42 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v 1.48 2000/07/31 22:39:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -26,8 +26,8 @@
 #include "utils/syscache.h"
 
 static char *getid(char *s, char *n);
-static int32 aclitemeq(AclItem *a1, AclItem *a2);
-static int32 aclitemgt(AclItem *a1, AclItem *a2);
+static bool aclitemeq(AclItem *a1, AclItem *a2);
+static bool aclitemgt(AclItem *a1, AclItem *a2);
 static char *aclparse(char *s, AclItem *aip, unsigned *modechg);
 
 #define ACL_IDTYPE_GID_KEYWORD	"group"
@@ -229,18 +229,14 @@ makeacl(int n)
  * RETURNS:
  *		the new AclItem
  */
-AclItem    *
-aclitemin(char *s)
+Datum
+aclitemin(PG_FUNCTION_ARGS)
 {
-	unsigned	modechg;
+	char	   *s = PG_GETARG_CSTRING(0);
 	AclItem    *aip;
-
-	if (!s)
-		elog(ERROR, "aclitemin: null string");
+	unsigned	modechg;
 
 	aip = (AclItem *) palloc(sizeof(AclItem));
-	if (!aip)
-		elog(ERROR, "aclitemin: palloc failed");
 	s = aclparse(s, aip, &modechg);
 	if (modechg != ACL_MODECHG_EQL)
 		elog(ERROR, "aclitemin: cannot accept anything but = ACLs");
@@ -248,7 +244,7 @@ aclitemin(char *s)
 		++s;
 	if (*s)
 		elog(ERROR, "aclitemin: extra garbage at end of specification");
-	return aip;
+	PG_RETURN_ACLITEM_P(aip);
 }
 
 /*
@@ -259,24 +255,17 @@ aclitemin(char *s)
  * RETURNS:
  *		the new string
  */
-char *
-aclitemout(AclItem *aip)
+Datum
+aclitemout(PG_FUNCTION_ARGS)
 {
+	AclItem	   *aip = PG_GETARG_ACLITEM_P(0);
 	char	   *p;
 	char	   *out;
 	HeapTuple	htup;
 	unsigned	i;
-	static AclItem default_aclitem = {ACL_ID_WORLD,
-		ACL_IDTYPE_WORLD,
-	ACL_WORLD_DEFAULT};
 	char	   *tmpname;
 
-	if (!aip)
-		aip = &default_aclitem;
-
 	p = out = palloc(strlen("group =arwR ") + 1 + NAMEDATALEN);
-	if (!out)
-		elog(ERROR, "aclitemout: palloc failed");
 	*p = '\0';
 
 	switch (aip->ai_idtype)
@@ -319,36 +308,28 @@ aclitemout(AclItem *aip)
 			*p++ = ACL_MODE_STR[i];
 	*p = '\0';
 
-	return out;
+	PG_RETURN_CSTRING(out);
 }
 
 /*
  * aclitemeq
  * aclitemgt
  *		AclItem equality and greater-than comparison routines.
- *		Two AclItems are equal iff they are both NULL or they have the
+ *		Two AclItems are equal iff they have the
  *		same identifier (and identifier type).
  *
  * RETURNS:
  *		a boolean value indicating = or >
  */
-static int32
+static bool
 aclitemeq(AclItem *a1, AclItem *a2)
 {
-	if (!a1 && !a2)
-		return 1;
-	if (!a1 || !a2)
-		return 0;
 	return a1->ai_idtype == a2->ai_idtype && a1->ai_id == a2->ai_id;
 }
 
-static int32
+static bool
 aclitemgt(AclItem *a1, AclItem *a2)
 {
-	if (a1 && !a2)
-		return 1;
-	if (!a1 || !a2)
-		return 0;
 	return ((a1->ai_idtype > a2->ai_idtype) ||
 			(a1->ai_idtype == a2->ai_idtype && a1->ai_id > a2->ai_id));
 }
@@ -384,25 +365,28 @@ acldefault(char *relname)
 	return acl;
 }
 
+/*
+ * Add or replace an item in an ACL array.
+ *
+ * NB: caller is responsible for having detoasted the input ACL, if needed.
+ */
 Acl *
 aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg)
 {
 	Acl		   *new_acl;
 	AclItem    *old_aip,
 			   *new_aip;
-	unsigned	src,
+	int			src,
 				dst,
 				num;
 
+	/* These checks for null input are probably dead code, but... */
 	if (!old_acl || ACL_NUM(old_acl) < 1)
-	{
-		new_acl = makeacl(0);
-		return new_acl;
-	}
+		old_acl = makeacl(0);
 	if (!mod_aip)
 	{
 		new_acl = makeacl(ACL_NUM(old_acl));
-		memmove((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
+		memcpy((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
 		return new_acl;
 	}
 
@@ -422,7 +406,7 @@ aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg)
 	{
 		/* modify in-place */
 		new_acl = makeacl(ACL_NUM(old_acl));
-		memmove((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
+		memcpy((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
 		new_aip = ACL_DAT(new_acl);
 		src = dst;
 	}
@@ -470,60 +454,58 @@ aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg)
 	/*
 	 * if the newly added entry has no permissions, delete it from the
 	 * list.  For example, this helps in removing entries for users who no
-	 * longer exists...
+	 * longer exist...
 	 */
-	for (dst = 1; dst < num; dst++)
+	if (new_aip[dst].ai_mode == 0)
 	{
-		if (new_aip[dst].ai_mode == 0)
-		{
-			int			i;
+		int			i;
 
-			for (i = dst + 1; i < num; i++)
-			{
-				new_aip[i - 1].ai_id = new_aip[i].ai_id;
-				new_aip[i - 1].ai_idtype = new_aip[i].ai_idtype;
-				new_aip[i - 1].ai_mode = new_aip[i].ai_mode;
-			}
-			ARR_DIMS(new_acl)[0] = num - 1;
-			/* Adjust also the array size because it is used for memmove */
-			ARR_SIZE(new_acl) -= sizeof(AclItem);
-			break;
+		for (i = dst + 1; i < num; i++)
+		{
+			new_aip[i - 1].ai_id = new_aip[i].ai_id;
+			new_aip[i - 1].ai_idtype = new_aip[i].ai_idtype;
+			new_aip[i - 1].ai_mode = new_aip[i].ai_mode;
 		}
+		ARR_DIMS(new_acl)[0] = num - 1;
+		/* Adjust also the array size because it is used for memmove */
+		ARR_SIZE(new_acl) -= sizeof(AclItem);
 	}
 
 	return new_acl;
 }
 
 /*
- * aclinsert
- *
+ * aclinsert (exported function)
  */
-Acl *
-aclinsert(Acl *old_acl, AclItem *mod_aip)
+Datum
+aclinsert(PG_FUNCTION_ARGS)
 {
-	return aclinsert3(old_acl, mod_aip, ACL_MODECHG_EQL);
+	Acl		   *old_acl = PG_GETARG_ACL_P(0);
+	AclItem	   *mod_aip = PG_GETARG_ACLITEM_P(1);
+
+	PG_RETURN_ACL_P(aclinsert3(old_acl, mod_aip, ACL_MODECHG_EQL));
 }
 
-Acl *
-aclremove(Acl *old_acl, AclItem *mod_aip)
+Datum
+aclremove(PG_FUNCTION_ARGS)
 {
+	Acl		   *old_acl = PG_GETARG_ACL_P(0);
+	AclItem	   *mod_aip = PG_GETARG_ACLITEM_P(1);
 	Acl		   *new_acl;
 	AclItem    *old_aip,
 			   *new_aip;
-	unsigned	dst,
+	int			dst,
 				old_num,
 				new_num;
 
+	/* These checks for null input should be dead code, but... */
 	if (!old_acl || ACL_NUM(old_acl) < 1)
-	{
-		new_acl = makeacl(0);
-		return new_acl;
-	}
+		old_acl = makeacl(0);
 	if (!mod_aip)
 	{
 		new_acl = makeacl(ACL_NUM(old_acl));
-		memmove((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
-		return new_acl;
+		memcpy((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
+		PG_RETURN_ACL_P(new_acl);
 	}
 
 	old_num = ACL_NUM(old_acl);
@@ -534,12 +516,12 @@ aclremove(Acl *old_acl, AclItem *mod_aip)
 	if (dst >= old_num)
 	{							/* not found or empty */
 		new_acl = makeacl(ACL_NUM(old_acl));
-		memmove((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
+		memcpy((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
 	}
 	else
 	{
 		new_num = old_num - 1;
-		new_acl = makeacl(ACL_NUM(old_acl) - 1);
+		new_acl = makeacl(new_num);
 		new_aip = ACL_DAT(new_acl);
 		if (dst == 0)
 		{						/* start */
@@ -561,23 +543,24 @@ aclremove(Acl *old_acl, AclItem *mod_aip)
 					(new_num - dst) * sizeof(AclItem));
 		}
 	}
-	return new_acl;
+	PG_RETURN_ACL_P(new_acl);
 }
 
-int32
-aclcontains(Acl *acl, AclItem *aip)
+Datum
+aclcontains(PG_FUNCTION_ARGS)
 {
-	unsigned	i,
-				num;
+	Acl		   *acl = PG_GETARG_ACL_P(0);
+	AclItem	   *aip = PG_GETARG_ACLITEM_P(1);
 	AclItem    *aidat;
+	int			i,
+				num;
 
-	if (!acl || !aip || ((num = ACL_NUM(acl)) < 1))
-		return 0;
+	num = ACL_NUM(acl);
 	aidat = ACL_DAT(acl);
 	for (i = 0; i < num; ++i)
 		if (aclitemeq(aip, aidat + i))
-			return 1;
-	return 0;
+			PG_RETURN_BOOL(true);
+	PG_RETURN_BOOL(false);
 }
 
 /* parser support routines */
@@ -638,7 +621,7 @@ aclmakepriv(char *old_privlist, char new_priv)
  *						"G"  - group
  *						"U"  - user
  *
- * concatentates the two strings together with a space in between
+ * concatenates the two strings together with a space in between
  *
  * this routine is used in the parser
  *
@@ -649,7 +632,7 @@ aclmakeuser(char *user_type, char *user)
 {
 	char	   *user_list;
 
-	user_list = palloc(strlen(user) + 3);
+	user_list = palloc(strlen(user_type) + strlen(user) + 2);
 	sprintf(user_list, "%s %s", user_type, user);
 	return user_list;
 }