Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 15 Aug 2008 08:47:29 GMT
From:      Edward Tomasz Napierala <trasz@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 147427 for review
Message-ID:  <200808150847.m7F8lT2v051456@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=147427

Change 147427 by trasz@trasz_traszkan on 2008/08/15 08:47:11

	Simplify the setfacl(1) utility, removing the acl[2] array
	and a few global variables.
	
	Note that the default ACL handling and mask recalculation
	were not tested.  Will do that later.

Affected files ...

.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/merge.c#9 edit
.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/remove.c#6 edit
.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/setfacl.c#11 edit
.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/setfacl.h#5 edit

Differences ...

==== //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/merge.c#9 (text+ko) ====

@@ -36,10 +36,11 @@
 
 #include "setfacl.h"
 
-static int merge_user_group(acl_entry_t *entry, acl_entry_t *entry_new);
+static int merge_user_group(acl_entry_t *entry, acl_entry_t *entry_new,
+    int acl_brand);
 
 static int
-merge_user_group(acl_entry_t *entry, acl_entry_t *entry_new)
+merge_user_group(acl_entry_t *entry, acl_entry_t *entry_new, int acl_brand)
 {
 	acl_permset_t permset;
 	acl_extended_t extended;
@@ -62,7 +63,7 @@
 		if (acl_set_permset(*entry_new, permset) == -1)
 			err(1, "acl_set_permset() failed");
 
-		if (acl_type == ACL_TYPE_NFS4) {
+		if (acl_brand == ACL_BRAND_NFS4) {
 			if (acl_get_extended_np(*entry, &extended))
 				err(1, "acl_get_extended_np() failed");
 			if (acl_set_extended_np(*entry_new, extended))
@@ -97,7 +98,7 @@
 	int acl_brand, prev_acl_brand;
 
 	acl_get_brand_np(acl, &acl_brand);
-	acl_get_brand_np(prev_acl[0], &prev_acl_brand);
+	acl_get_brand_np(*prev_acl, &prev_acl_brand);
 
 	if (acl_brand != prev_acl_brand) {
 		warnx("%s: branding mismatch; existing ACL is %s, "
@@ -107,10 +108,7 @@
 		return (-1);
 	}
 
-	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4)
-		acl_new = acl_dup(prev_acl[ACCESS_ACL]);
-	else
-		acl_new = acl_dup(prev_acl[DEFAULT_ACL]);
+	acl_new = acl_dup(*prev_acl);
 	if (acl_new == NULL)
 		err(1, "%s: acl_dup() failed", filename);
 
@@ -145,7 +143,7 @@
 			 * For NFS4, in addition to "tag" and "id" we also
 			 * compare "extended".
 			 */
-			if (acl_type == ACL_TYPE_NFS4) {
+			if (acl_brand == ACL_BRAND_NFS4) {
 				if (acl_get_extended_np(entry, &extended))
 					err(1, "%s: acl_get_extended_np() "
 					    "failed", filename);
@@ -160,7 +158,7 @@
 			case ACL_USER:
 			case ACL_GROUP:
 				have_entry = merge_user_group(&entry,
-				    &entry_new);
+				    &entry_new, acl_brand);
 				if (have_entry == 0)
 					break;
 				/* FALLTHROUGH */
@@ -176,7 +174,7 @@
 					err(1, "%s: acl_set_permset() failed",
 					    filename);
 
-				if (acl_type == ACL_TYPE_NFS4) {
+				if (acl_brand == ACL_BRAND_NFS4) {
 					if (acl_get_extended_np(entry, &extended))
 						err(1, "%s: acl_get_extended_np() failed",
 						    filename);
@@ -207,7 +205,7 @@
 			 * Appending them at the end makes no sense, since
 			 * in most cases they wouldn't even get evaluated.
 			 */
-			if (acl_type == ACL_TYPE_NFS4) {
+			if (acl_brand == ACL_BRAND_NFS4) {
 				if (acl_create_entry_np(&acl_new, &entry_new, entry_number) == -1) {
 					warn("%s: acl_create_entry_np() failed", filename); 
 					acl_free(acl_new);
@@ -232,13 +230,8 @@
 		}
 	}
 
-	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4) {
-		acl_free(prev_acl[ACCESS_ACL]);
-		prev_acl[ACCESS_ACL] = acl_new;
-	} else {
-		acl_free(prev_acl[DEFAULT_ACL]);
-		prev_acl[DEFAULT_ACL] = acl_new;
-	}
+	acl_free(*prev_acl);
+	*prev_acl = acl_new;
 
 	return (0);
 }
@@ -248,24 +241,25 @@
 {
 	acl_entry_t entry, entry_new;
 	acl_t acl_new;
-	int entry_id, acl_brand;
+	int entry_id, acl_brand, prev_acl_brand;
+
+	acl_get_brand_np(acl, &acl_brand);
+	acl_get_brand_np(*prev_acl, &prev_acl_brand);
 
-	if (acl_type != ACL_TYPE_NFS4) {
+	if (prev_acl_brand != ACL_BRAND_NFS4) {
 		warnx("%s: the '-a' option is only applicable to NFS4 ACLs",
 		    filename);
 		return (-1);
 	}
 
-	acl_get_brand_np(acl, &acl_brand);
-
 	if (acl_brand != ACL_BRAND_NFS4) {
-		warnx("%s: branding mismatch; existing ACL is %s, "
-		    "entry to be added is NFS4", filename,
+		warnx("%s: branding mismatch; existing ACL is NFS4, "
+		    "entry to be added is %s", filename,
 		    acl_brand == ACL_BRAND_NFS4 ? "NFS4" : "POSIX");
 		return (-1);
 	}
 
-	acl_new = acl_dup(prev_acl[ACCESS_ACL]);
+	acl_new = acl_dup(*prev_acl);
 	if (acl_new == NULL)
 		err(1, "%s: acl_dup() failed", filename);
 
@@ -295,8 +289,8 @@
 			err(1, "%s: acl_copy_entry() failed", filename);
 	}
 
-	acl_free(prev_acl[ACCESS_ACL]);
-	prev_acl[ACCESS_ACL] = acl_new;
+	acl_free(*prev_acl);
+	*prev_acl = acl_new;
 
 	return (0);
 }

==== //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/remove.c#6 (text+ko) ====

@@ -51,7 +51,7 @@
 	carried_error = 0;
 
 	acl_get_brand_np(acl, &acl_brand);
-	acl_get_brand_np(prev_acl[0], &prev_acl_brand);
+	acl_get_brand_np(*prev_acl, &prev_acl_brand);
 
 	if (acl_brand != prev_acl_brand) {
 		warnx("%s: branding mismatch; existing ACL is %s, "
@@ -63,10 +63,7 @@
 
 	carried_error = 0;
 
-	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4)
-		acl_new = acl_dup(prev_acl[ACCESS_ACL]);
-	else
-		acl_new = acl_dup(prev_acl[DEFAULT_ACL]);
+	acl_new = acl_dup(*prev_acl);
 	if (acl_new == NULL)
 		err(1, "%s: acl_dup() failed", filename);
 
@@ -87,13 +84,8 @@
 		}
 	}
 
-	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4) {
-		acl_free(prev_acl[ACCESS_ACL]);
-		prev_acl[ACCESS_ACL] = acl_new;
-	} else {
-		acl_free(prev_acl[DEFAULT_ACL]);
-		prev_acl[DEFAULT_ACL] = acl_new;
-	}
+	acl_free(*prev_acl);
+	*prev_acl = acl_new;
 
 	if (carried_error)
 		return (-1);
@@ -112,10 +104,7 @@
 
 	carried_error = 0;
 
-	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4)
-		acl_new = acl_dup(prev_acl[ACCESS_ACL]);
-	else
-		acl_new = acl_dup(prev_acl[DEFAULT_ACL]);
+	acl_new = acl_dup(*prev_acl);
 	if (acl_new == NULL)
 		err(1, "%s: acl_dup() failed", filename);
 
@@ -148,13 +137,8 @@
 			warn("%s: acl_delete_entry_np() failed", filename);
 	}
 
-	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4) {
-		acl_free(prev_acl[ACCESS_ACL]);
-		prev_acl[ACCESS_ACL] = acl_new;
-	} else {
-		acl_free(prev_acl[DEFAULT_ACL]);
-		prev_acl[DEFAULT_ACL] = acl_new;
-	}
+	acl_free(*prev_acl);
+	*prev_acl = acl_new;
 
 	if (carried_error)
 		return (-1);
@@ -168,16 +152,11 @@
 int
 remove_default(acl_t *prev_acl, const char *filename)
 {
+	acl_free(*prev_acl);
+	*prev_acl = acl_init(ACL_MAX_ENTRIES);
+	if (*prev_acl == NULL)
+		err(1, "%s: acl_init() failed", filename);
 
-	if (prev_acl[1]) {
-		acl_free(prev_acl[1]);
-		prev_acl[1] = acl_init(ACL_MAX_ENTRIES);
-		if (prev_acl[1] == NULL)
-			err(1, "%s: acl_init() failed", filename);
-	} else {
-		warn("%s: cannot remove default ACL", filename);
-		return (-1);
-	}
 	return (0);
 }
 
@@ -187,22 +166,13 @@
 void
 remove_ext(acl_t *prev_acl, const char *filename)
 {
-	acl_t acl_new, acl_old;
+	acl_t acl_new;
 
-	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4)
-		acl_old = prev_acl[ACCESS_ACL];
-	else
-		acl_old = prev_acl[DEFAULT_ACL];
-
-	acl_new = acl_strip_np(acl_old, !n_flag);
+	acl_new = acl_strip_np(*prev_acl, !n_flag);
 	if (acl_new == NULL)
 		err(1, "%s: acl_strip_np() failed", filename);
 
-	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4) {
-		acl_free(prev_acl[ACCESS_ACL]);
-		prev_acl[ACCESS_ACL] = acl_new;
-	} else {
-		acl_free(prev_acl[DEFAULT_ACL]);
-		prev_acl[DEFAULT_ACL] = acl_new;
-	}
+	acl_free(*prev_acl);
+	*prev_acl = acl_new;
 }
+

==== //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/setfacl.c#11 (text+ko) ====

@@ -41,9 +41,9 @@
 
 #include "setfacl.h"
 
-static void   add_filename(const char *filename);
-static acl_t *get_file_acls(const char *filename);
-static void   usage(void);
+static void	add_filename(const char *filename);
+static acl_t	get_file_acl(const char *filename, acl_type_t type, int h_flag);
+static void	usage(void);
 
 static void
 add_filename(const char *filename)
@@ -59,41 +59,55 @@
 	TAILQ_INSERT_TAIL(&filelist, file, next);
 }
 
-static acl_t *
-get_file_acls(const char *filename)
+static acl_t
+get_file_acl(const char *filename, acl_type_t type, int h_flag)
 {
-	acl_t *acl;
+	acl_t acl = NULL;
 	struct stat sb;
-	int type;
 
-	if (stat(filename, &sb) == -1) {
-		warn("%s: stat() failed", filename);
-		return (NULL);
+	if (pathconf(filename, _PC_EXTENDED_SECURITY_NP)) {
+		if (type == ACL_TYPE_ACCESS) {
+			type = ACL_TYPE_NFS4;
+		} else {
+			warnx("%s: there are no default entries in NFS4 ACLs",
+			    filename);
+			return (NULL);
+		}
 	}
 
-	if (pathconf(filename, _PC_EXTENDED_SECURITY_NP))
-		type = ACL_TYPE_NFS4;
-	else
-		type = ACL_TYPE_ACCESS;
+	switch (type) {
+	case ACL_TYPE_ACCESS:
+	case ACL_TYPE_NFS4:
+		if (h_flag)
+			acl = acl_get_link_np(filename, type);
+		else
+			acl = acl_get_file(filename, type);
+		break;
+
+	case ACL_TYPE_DEFAULT:
+		if (stat(filename, &sb) == -1) {
+			warn("%s: stat() failed", filename);
+			return (NULL);
+		}
+
+		if (S_ISDIR(sb.st_mode) == 0)
+			return (NULL);
+
+		if (h_flag)
+			acl = acl_get_link_np(filename, ACL_TYPE_DEFAULT);
+		else
+			acl = acl_get_file(filename, ACL_TYPE_DEFAULT);
+		break;
+	}
 
-	acl = zmalloc(sizeof(acl_t) * 2);
-	if (h_flag)
-		acl[ACCESS_ACL] = acl_get_link_np(filename, type);
-	else
-		acl[ACCESS_ACL] = acl_get_file(filename, type);
-	if (acl[ACCESS_ACL] == NULL)
-		err(1, "%s: acl_get_file() failed", filename);
-	if (S_ISDIR(sb.st_mode) && type != ACL_TYPE_NFS4) {
+	if (acl == NULL) {
 		if (h_flag)
-			acl[DEFAULT_ACL] = acl_get_link_np(filename,
-			    ACL_TYPE_DEFAULT);
+			warn("%s: acl_get_link_np() failed", filename);
 		else
-			acl[DEFAULT_ACL] = acl_get_file(filename,
-			    ACL_TYPE_DEFAULT);
-		if (acl[DEFAULT_ACL] == NULL)
-			err(1, "%s: acl_get_file() failed", filename);
-	} else
-		acl[DEFAULT_ACL] = NULL;
+			warn("%s: acl_get_file() failed", filename);
+
+		return (NULL);
+	}
 
 	return (acl);
 }
@@ -110,9 +124,11 @@
 int
 main(int argc, char *argv[])
 {
-	acl_t *acl, final_acl;
+	acl_t acl;
+	acl_type_t acl_type;
 	char filename[PATH_MAX];
 	int local_error, carried_error, ch, i, entry_number;
+	int h_flag;
 	struct sf_file *file;
 	struct sf_entry *entry;
 	const char *fn_dup;
@@ -233,17 +249,9 @@
 
 	/* cycle through each file */
 	TAILQ_FOREACH(file, &filelist, next) {
-		/* get our initial access and default ACL's */
-		acl = get_file_acls(file->filename);
+		acl = get_file_acl(file->filename, acl_type, h_flag);
 		if (acl == NULL)
 			continue;
-		if ((acl_type == ACL_TYPE_DEFAULT) && !acl[1]) {
-			if (pathconf(file->filename, _PC_EXTENDED_SECURITY_NP))
-				warnx("%s: there are no default entries in NFS4 ACLs", file->filename);
-			else
-				warnx("%s: default ACL not valid", file->filename);
-			continue;
-		}
 
 		local_error = 0;
 
@@ -260,15 +268,15 @@
 			switch(entry->op) {
 			case OP_ADD_ACL:
 				local_error += add_acl(entry->acl,
-				    entry->entry_number, acl, file->filename);
+				    entry->entry_number, &acl, file->filename);
 				break;
 			case OP_MERGE_ACL:
-				local_error += merge_acl(entry->acl, acl,
+				local_error += merge_acl(entry->acl, &acl,
 				    file->filename);
 				need_mask = 1;
 				break;
 			case OP_REMOVE_EXT:
-				remove_ext(acl, file->filename);
+				remove_ext(&acl, file->filename);
 				need_mask = 0;
 				break;
 			case OP_REMOVE_DEF:
@@ -283,17 +291,19 @@
 					    file->filename);
 					local_error++;
 				}
-				local_error += remove_default(acl, file->filename);
+				if (acl_type == ACL_TYPE_DEFAULT)
+					local_error += remove_default(&acl,
+					    file->filename);
 				need_mask = 0;
 				break;
 			case OP_REMOVE_ACL:
-				local_error += remove_acl(entry->acl, acl,
+				local_error += remove_acl(entry->acl, &acl,
 				    file->filename);
 				need_mask = 1;
 				break;
 			case OP_REMOVE_BY_NUMBER:
 				local_error += remove_by_number(entry->entry_number,
-				    acl, file->filename);
+				    &acl, file->filename);
 				need_mask = 1;
 				break;
 			}
@@ -305,43 +315,32 @@
 			continue;
 		}
 
-		if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4)
-			final_acl = acl[ACCESS_ACL];
-		else
-			final_acl = acl[DEFAULT_ACL];
-
 		if (acl_type == ACL_TYPE_NFS4)
 			need_mask = 0;
 
-		if (need_mask && (set_acl_mask(&final_acl,
+		if (need_mask && (set_acl_mask(&acl,
 		    file->filename) == -1)) {
 			warnx("%s: failed to set ACL mask", file->filename);
 			carried_error++;
 		} else if (h_flag) {
 			if (acl_set_link_np(file->filename, acl_type,
-			    final_acl) == -1) {
+			    acl) == -1) {
 				carried_error++;
 				warn("%s: acl_set_link_np() failed",
 				    file->filename);
 			}
 		} else {
 			if (acl_set_file(file->filename, acl_type,
-			    final_acl) == -1) {
+			    acl) == -1) {
 				carried_error++;
 				warn("%s: acl_set_file() failed",
 				    file->filename);
 			}
 		}
 
-		if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4)
-			acl[ACCESS_ACL] = final_acl;
-		else
-			acl[DEFAULT_ACL] = final_acl;
-
-		acl_free(acl[ACCESS_ACL]);
-		acl_free(acl[DEFAULT_ACL]);
-		free(acl);
+		acl_free(acl);
 	}
 
 	return (carried_error);
 }
+

==== //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/setfacl.h#5 (text+ko) ====

@@ -41,10 +41,6 @@
 #define OP_REMOVE_BY_NUMBER	0x04	/* remove acl's (-xX) by acl entry number */
 #define OP_ADD_ACL		0x05	/* add acls entries at a given position */
 
-/* ACL types for the acl array */
-#define ACCESS_ACL	0
-#define DEFAULT_ACL	1
-
 /* TAILQ entry for acl operations */
 struct sf_entry {
 	uint	op;
@@ -76,11 +72,9 @@
 /* util.c */
 void  *zmalloc(size_t size);
 
-acl_type_t acl_type;
 uint       have_mask;
 uint       need_mask;
 uint       have_stdin;
-uint       h_flag;
 uint       n_flag;
 
 #endif /* _SETFACL_H */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200808150847.m7F8lT2v051456>