Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 7 Aug 2008 11:55:40 GMT
From:      Edward Tomasz Napierala <trasz@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 146831 for review
Message-ID:  <200808071155.m77Bteek031358@repoman.freebsd.org>

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

Change 146831 by trasz@trasz_traszkan on 2008/08/07 11:55:12

	Prepend errors printed out by setfacl(1) with file name.  Use
	the same capitalization rules in all messages.

Affected files ...

.. //depot/projects/soc2008/trasz_nfs4acl/TODO#35 edit
.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/mask.c#2 edit
.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/merge.c#7 edit
.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/remove.c#5 edit
.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/setfacl.c#7 edit
.. //depot/projects/soc2008/trasz_nfs4acl/bin/setfacl/setfacl.h#4 edit

Differences ...

==== //depot/projects/soc2008/trasz_nfs4acl/TODO#35 (text+ko) ====

@@ -1,7 +1,5 @@
 Things to do, in no particular order:
 
-- Make setfacl(1) error messages more user friendly.
-
 - Add support for NFS4 ACLs to tar(1).
 
 - Clean up #defines.  For example, make VREAD_NAMED_ATTRS equal

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

@@ -40,7 +40,7 @@
 
 /* set the appropriate mask the given ACL's */
 int
-set_acl_mask(acl_t *prev_acl)
+set_acl_mask(acl_t *prev_acl, char *filename)
 {
 	acl_entry_t entry;
 	acl_t acl;
@@ -59,7 +59,7 @@
 
 	acl = acl_dup(*prev_acl);
 	if (acl == NULL)
-		err(1, "acl_dup() failed");
+		err(1, "%s: acl_dup() failed", filename);
 
 	if (n_flag == 0) {
 		/*
@@ -70,7 +70,7 @@
 		 * class in the resulting ACL
 		 */
 		if (acl_calc_mask(&acl)) {
-			warn("acl_calc_mask() failed");
+			warn("%s: acl_calc_mask() failed", filename);
 			acl_free(acl);
 			return (-1);
 		}
@@ -86,7 +86,8 @@
 		while (acl_get_entry(acl, entry_id, &entry) == 1) {
 			entry_id = ACL_NEXT_ENTRY;
 			if (acl_get_tag_type(entry, &tag) == -1)
-				err(1, "acl_get_tag_type() failed");
+				err(1, "%s: acl_get_tag_type() failed",
+				    filename);
 
 			if (tag == ACL_MASK) {
 				acl_free(acl);
@@ -100,7 +101,7 @@
 		 * file, then write an error message to standard error and
 		 * continue with the next file.
 		 */
-		warnx("warning: no mask entry");
+		warnx("%s: warning: no mask entry", filename);
 		acl_free(acl);
 		return (0);
 	}

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

@@ -85,7 +85,7 @@
  * merge an ACL into existing file's ACL
  */
 int
-merge_acl(acl_t acl, acl_t *prev_acl)
+merge_acl(acl_t acl, acl_t *prev_acl, const char *filename)
 {
 	acl_entry_t entry, entry_new;
 	acl_permset_t permset;
@@ -100,8 +100,8 @@
 	acl_get_brand_np(prev_acl[0], &prev_acl_brand);
 
 	if (acl_brand != prev_acl_brand) {
-		warnx("branding mismatch; existing ACL is %s, "
-		    "entry to be merged is %s",
+		warnx("%s: branding mismatch; existing ACL is %s, "
+		    "entry to be merged is %s", filename,
 		    prev_acl_brand == ACL_BRAND_NFS4 ? "NFS4" : "POSIX",
 		    acl_brand == ACL_BRAND_NFS4 ? "NFS4" : "POSIX");
 		return (-1);
@@ -112,7 +112,7 @@
 	else
 		acl_new = acl_dup(prev_acl[DEFAULT_ACL]);
 	if (acl_new == NULL)
-		err(1, "acl_dup() failed");
+		err(1, "%s: acl_dup() failed", filename);
 
 	entry_id = ACL_FIRST_ENTRY;
 
@@ -122,7 +122,8 @@
 
 		/* keep track of existing ACL_MASK entries */
 		if (acl_get_tag_type(entry, &tag) == -1)
-			err(1, "acl_get_tag_type() failed - invalid ACL entry");
+			err(1, "%s: acl_get_tag_type() failed - "
+			    "invalid ACL entry", filename);
 		if (tag == ACL_MASK)
 			have_mask = 1;
 
@@ -133,9 +134,11 @@
 			entry_id_new = ACL_NEXT_ENTRY;
 
 			if (acl_get_tag_type(entry, &tag) == -1)
-				err(1, "acl_get_tag_type() failed");
+				err(1, "%s: acl_get_tag_type() failed",
+				    filename);
 			if (acl_get_tag_type(entry_new, &tag_new) == -1)
-				err(1, "acl_get_tag_type() failed");
+				err(1, "%s: acl_get_tag_type() failed",
+				    filename);
 			if (tag != tag_new)
 				continue;
 
@@ -145,9 +148,11 @@
 			 */
 			if (acl_type == ACL_TYPE_NFS4) {
 				if (acl_get_extended_np(entry, &extended))
-					err(1, "acl_get_extended_np() failed");
+					err(1, "%s: acl_get_extended_np() "
+					    "failed", filename);
 				if (acl_get_extended_np(entry_new, &extended_new))
-					err(1, "acl_get_extended_np() failed");
+					err(1, "%s: acl_get_extended_np() "
+					    "failed", filename);
 				if (extended != extended_new)
 					continue;
 			}
@@ -166,25 +171,31 @@
 			case ACL_MASK:
 			case ACL_EVERYONE:
 				if (acl_get_permset(entry, &permset) == -1)
-					err(1, "acl_get_permset() failed");
+					err(1, "%s: acl_get_permset() failed",
+					    filename);
 				if (acl_set_permset(entry_new, permset) == -1)
-					err(1, "acl_set_permset() failed");
+					err(1, "%s: acl_set_permset() failed",
+					    filename);
 
 				if (acl_type == ACL_TYPE_NFS4) {
 					if (acl_get_extended_np(entry, &extended))
-						err(1, "acl_get_extended_np() failed");
+						err(1, "%s: acl_get_extended_np() failed",
+						    filename);
 					if (acl_set_extended_np(entry_new, extended))
-						err(1, "acl_set_extended_np() failed");
+						err(1, "%s: acl_set_extended_np() failed",
+						    filename);
 					if (acl_get_flagset_np(entry, &flagset))
-						err(1, "acl_get_flagset_np() failed");
+						err(1, "%s: acl_get_flagset_np() failed",
+						    filename);
 					if (acl_set_flagset_np(entry_new, flagset))
-						err(1, "acl_set_flagset_np() failed");
+						err(1, "%s: acl_set_flagset_np() failed",
+						    filename);
 				}
 				have_entry = 1;
 				break;
 			default:
 				/* should never be here */
-				errx(1, "Invalid tag type: %i", tag);
+				errx(1, "%s: invalid tag type: %i", filename, tag);
 				break;
 			}
 		}
@@ -199,7 +210,7 @@
 			 */
 			if (acl_type == ACL_TYPE_NFS4) {
 				if (acl_create_entry_np(&acl_new, &entry_new, entry_number) == -1) {
-					warnx("acl_create_entry_np() failed"); 
+					warn("%s: acl_create_entry_np() failed", filename); 
 					acl_free(acl_new);
 					return (-1);
 				}
@@ -212,13 +223,13 @@
 				entry_number++;
 			} else {
 				if (acl_create_entry(&acl_new, &entry_new) == -1) {
-					warnx("acl_create_entry() failed"); 
+					warn("%s: acl_create_entry() failed", filename); 
 					acl_free(acl_new);
 					return (-1);
 				}
 			}
 			if (acl_copy_entry(entry_new, entry) == -1)
-				err(1, "acl_copy_entry() failed");
+				err(1, "%s: acl_copy_entry() failed", filename);
 		}
 	}
 
@@ -234,29 +245,30 @@
 }
 
 int
-add_acl(acl_t acl, uint entry_number, acl_t *prev_acl)
+add_acl(acl_t acl, uint entry_number, acl_t *prev_acl, const char *filename)
 {
 	acl_entry_t entry, entry_new;
 	acl_t acl_new;
 	int entry_id, acl_brand;
 
 	if (acl_type != ACL_TYPE_NFS4) {
-		warnx("The '-a' option is only applicable to NFS4 ACLs");
+		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("branding mismatch; existing ACL is %s, "
-		    "entry to be added is NFS4",
+		warnx("%s: branding mismatch; existing ACL is %s, "
+		    "entry to be added is NFS4", filename,
 		    acl_brand == ACL_BRAND_NFS4 ? "NFS4" : "POSIX");
 		return (-1);
 	}
 
 	acl_new = acl_dup(prev_acl[ACCESS_ACL]);
 	if (acl_new == NULL)
-		err(1, "acl_dup() failed");
+		err(1, "%s: acl_dup() failed", filename);
 
 	entry_id = ACL_FIRST_ENTRY;
 
@@ -264,7 +276,10 @@
 		entry_id = ACL_NEXT_ENTRY;
 
 		if (acl_create_entry_np(&acl_new, &entry_new, entry_number) == -1) {
-			warnx("acl_create_entry_np() failed"); 
+			if (entry_number >= (uint)acl_new->ats_acl.acl_cnt)
+				warnx("%s: invalid entry number", filename);
+			else
+				warn("%s: acl_create_entry_np() failed", filename); 
 			acl_free(acl_new);
 			return (-1);
 		}
@@ -278,7 +293,7 @@
 		entry_number++;
 
 		if (acl_copy_entry(entry_new, entry) == -1)
-			err(1, "acl_copy_entry() failed");
+			err(1, "%s: acl_copy_entry() failed", filename);
 	}
 
 	acl_free(prev_acl[ACCESS_ACL]);

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

@@ -41,7 +41,7 @@
  * remove ACL entries from an ACL
  */
 int
-remove_acl(acl_t acl, acl_t *prev_acl)
+remove_acl(acl_t acl, acl_t *prev_acl, const char *filename)
 {
 	acl_entry_t	entry;
 	acl_t		acl_new;
@@ -54,8 +54,8 @@
 	acl_get_brand_np(prev_acl[0], &prev_acl_brand);
 
 	if (acl_brand != prev_acl_brand) {
-		warnx("branding mismatch; existing ACL is %s, "
-		    "entry to be removed is %s",
+		warnx("%s: branding mismatch; existing ACL is %s, "
+		    "entry to be removed is %s", filename,
 		    prev_acl_brand == ACL_BRAND_NFS4 ? "NFS4" : "POSIX",
 		    acl_brand == ACL_BRAND_NFS4 ? "NFS4" : "POSIX");
 		return (-1);
@@ -68,7 +68,7 @@
 	else
 		acl_new = acl_dup(prev_acl[DEFAULT_ACL]);
 	if (acl_new == NULL)
-		err(1, "acl_dup() failed");
+		err(1, "%s: acl_dup() failed", filename);
 
 	tag = ACL_UNDEFINED_TAG;
 
@@ -77,12 +77,13 @@
 	while (acl_get_entry(acl, entry_id, &entry) == 1) {
 		entry_id = ACL_NEXT_ENTRY;
 		if (acl_get_tag_type(entry, &tag) == -1)
-			err(1, "acl_get_tag_type() failed");
+			err(1, "%s: acl_get_tag_type() failed", filename);
 		if (tag == ACL_MASK)
 			have_mask++;
 		if (acl_delete_entry(acl_new, entry) == -1) {
 			carried_error++;
-			warnx("cannot remove non-existent acl entry");
+			warnx("%s: cannot remove non-existent ACL entry",
+			    filename);
 		}
 	}
 
@@ -101,7 +102,7 @@
 }
 
 int
-remove_by_number(uint entry_number, acl_t *prev_acl)
+remove_by_number(uint entry_number, acl_t *prev_acl, const char *filename)
 {
 	acl_entry_t	entry;
 	acl_t		acl_new;
@@ -116,7 +117,7 @@
 	else
 		acl_new = acl_dup(prev_acl[DEFAULT_ACL]);
 	if (acl_new == NULL)
-		err(1, "acl_dup() failed");
+		err(1, "%s: acl_dup() failed", filename);
 
 	tag = ACL_UNDEFINED_TAG;
 
@@ -133,14 +134,18 @@
 		if (i != entry_number)
 			continue;
 		if (acl_get_tag_type(entry, &tag) == -1)
-			err(1, "acl_get_tag_type() failed");
+			err(1, "%s: acl_get_tag_type() failed", filename);
 		if (tag == ACL_MASK)
 			have_mask++;
 	}
 
 	if (acl_delete_entry_np(acl_new, entry_number) == -1) {
 		carried_error++;
-		warnx("cannot remove non-existent acl entry");
+
+		if (entry_number >= (uint)acl_new->ats_acl.acl_cnt)
+			warnx("%s: invalid entry number", filename);
+		else
+			warn("%s: acl_delete_entry_np() failed", filename);
 	}
 
 	if (acl_type == ACL_TYPE_ACCESS || acl_type == ACL_TYPE_NFS4) {
@@ -161,16 +166,16 @@
  * remove default entries
  */
 int
-remove_default(acl_t *prev_acl)
+remove_default(acl_t *prev_acl, const char *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, "acl_init() failed");
+			err(1, "%s: acl_init() failed", filename);
 	} else {
-		warn("cannot remove default ACL");
+		warn("%s: cannot remove default ACL", filename);
 		return (-1);
 	}
 	return (0);
@@ -180,7 +185,7 @@
  * remove extended entries
  */
 void
-remove_ext(acl_t *prev_acl)
+remove_ext(acl_t *prev_acl, const char *filename)
 {
 	acl_t acl_new, acl_old;
 
@@ -191,7 +196,7 @@
 
 	acl_new = acl_strip_np(acl_old, !n_flag);
 	if (acl_new == NULL)
-		err(1, "acl_strip_np() failed");
+		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]);

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

@@ -67,7 +67,7 @@
 	int type;
 
 	if (stat(filename, &sb) == -1) {
-		warn("stat() of %s failed", filename);
+		warn("%s: stat() failed", filename);
 		return (NULL);
 	}
 
@@ -82,7 +82,7 @@
 	else
 		acl[ACCESS_ACL] = acl_get_file(filename, type);
 	if (acl[ACCESS_ACL] == NULL)
-		err(1, "acl_get_file() failed");
+		err(1, "%s: acl_get_file() failed", filename);
 	if (S_ISDIR(sb.st_mode) && type != ACL_TYPE_NFS4) {
 		if (h_flag)
 			acl[DEFAULT_ACL] = acl_get_link_np(filename,
@@ -91,7 +91,7 @@
 			acl[DEFAULT_ACL] = acl_get_file(filename,
 			    ACL_TYPE_DEFAULT);
 		if (acl[DEFAULT_ACL] == NULL)
-			err(1, "acl_get_file() failed");
+			err(1, "%s: acl_get_file() failed", filename);
 	} else
 		acl[DEFAULT_ACL] = NULL;
 
@@ -131,7 +131,7 @@
 			entry = zmalloc(sizeof(struct sf_entry));
 			entry->acl = get_acl_from_file(optarg);
 			if (entry->acl == NULL)
-				err(1, "get_acl_from_file() failed");
+				err(1, "%s: get_acl_from_file() failed", optarg);
 			entry->op = OP_MERGE_ACL;
 			TAILQ_INSERT_TAIL(&entrylist, entry, next);
 			break;
@@ -146,13 +146,13 @@
 
 			entry_number = strtol(optarg, &end, 10);
 			if (end - optarg != (int)strlen(optarg))
-				errx(1, "%s: Invalid entry number", optarg);
+				errx(1, "%s: invalid entry number", optarg);
 			if (entry_number < 0)
-				errx(1, "%s: Entry number cannot be less than zero", optarg);
+				errx(1, "%s: entry number cannot be less than zero", optarg);
 			entry->entry_number = entry_number;
 
 			if (argv[optind] == NULL)
-				errx(1, "Missing ACL");
+				errx(1, "missing ACL");
 			entry->acl = acl_from_text(argv[optind]);
 			if (entry->acl == NULL)
 				err(1, "%s", argv[optind]);
@@ -192,7 +192,7 @@
 			entry_number = strtol(optarg, &end, 10);
 			if (end - optarg == (int)strlen(optarg)) {
 				if (entry_number < 0)
-					errx(1, "%s: Entry number cannot be less than zero", optarg);
+					errx(1, "%s: entry number cannot be less than zero", optarg);
 				entry->entry_number = entry_number;
 				entry->op = OP_REMOVE_BY_NUMBER;
 			} else {
@@ -239,9 +239,9 @@
 			continue;
 		if ((acl_type == ACL_TYPE_DEFAULT) && !acl[1]) {
 			if (pathconf(file->filename, _PC_EXTENDED_SECURITY_NP))
-				warnx("there are no default entries in NFS4 ACLs: %s", file->filename);
+				warnx("%s: there are no default entries in NFS4 ACLs", file->filename);
 			else
-				warnx("default ACL not valid for %s", file->filename);
+				warnx("%s: default ACL not valid", file->filename);
 			continue;
 		}
 
@@ -259,35 +259,41 @@
 
 			switch(entry->op) {
 			case OP_ADD_ACL:
-				local_error += add_acl(entry->acl, entry->entry_number, acl);
+				local_error += add_acl(entry->acl,
+				    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);
+				remove_ext(acl, file->filename);
 				need_mask = 0;
 				break;
 			case OP_REMOVE_DEF:
 				if (acl_type == ACL_TYPE_NFS4) {
-					warnx("there are no default entries in NFS4 ACLs; cannot remove");
+					warnx("%s: there are no default entries in NFS4 ACLs; "
+					    "cannot remove", file->filename);
 					local_error++;
 					break;
 				}
 				if (acl_delete_def_file(file->filename) == -1) {
-					warn("acl_delete_def_file() failed");
+					warn("%s: acl_delete_def_file() failed",
+					    file->filename);
 					local_error++;
 				}
-				local_error += remove_default(acl);
+				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);
+				local_error += remove_by_number(entry->entry_number,
+				    acl, file->filename);
 				need_mask = 1;
 				break;
 			}
@@ -307,21 +313,21 @@
 		if (acl_type == ACL_TYPE_NFS4)
 			need_mask = 0;
 
-		if (need_mask && (set_acl_mask(&final_acl) == -1)) {
-			warnx("failed to set ACL mask on %s", file->filename);
+		if (need_mask && (set_acl_mask(&final_acl, 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) {
 				carried_error++;
-				warn("acl_set_link_np() failed for %s",
+				warn("%s: acl_set_link_np() failed",
 				    file->filename);
 			}
 		} else {
 			if (acl_set_file(file->filename, acl_type,
 			    final_acl) == -1) {
 				carried_error++;
-				warn("acl_set_file() failed for %s",
+				warn("%s: acl_set_file() failed",
 				    file->filename);
 			}
 		}

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

@@ -64,15 +64,15 @@
 /* files.c */
 acl_t  get_acl_from_file(const char *filename);
 /* merge.c */
-int    merge_acl(acl_t acl, acl_t *prev_acl);
-int    add_acl(acl_t acl, uint entry_number, acl_t *prev_acl);
+int    merge_acl(acl_t acl, acl_t *prev_acl, const char *filename);
+int    add_acl(acl_t acl, uint entry_number, acl_t *prev_acl, const char *filename);
 /* remove.c */
-int    remove_acl(acl_t acl, acl_t *prev_acl);
-int    remove_by_number(uint entry_number, acl_t *prev_acl);
-int    remove_default(acl_t *prev_acl);
-void   remove_ext(acl_t *prev_acl);
+int    remove_acl(acl_t acl, acl_t *prev_acl, const char *filename);
+int    remove_by_number(uint entry_number, acl_t *prev_acl, const char *filename);
+int    remove_default(acl_t *prev_acl, const char *filename);
+void   remove_ext(acl_t *prev_acl, const char *filename);
 /* mask.c */
-int    set_acl_mask(acl_t *prev_acl);
+int    set_acl_mask(acl_t *prev_acl, const char *filename);
 /* util.c */
 void  *zmalloc(size_t size);
 



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