Date: Mon, 28 Jul 2008 13:38:54 GMT From: Edward Tomasz Napierala <trasz@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 146118 for review Message-ID: <200807281338.m6SDcsuN013197@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=146118 Change 146118 by trasz@trasz_traszkan on 2008/07/28 13:37:57 Rework branding, as suggested by rwatson@. There should be no externally visible changes. Affected files ... .. //depot/projects/soc2008/trasz_nfs4acl/TODO#16 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_branding.c#4 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_calc_mask.c#3 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_copy.c#3 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_delete_entry.c#5 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_flag.c#2 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_from_text.c#4 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_from_text_nfs4.c#3 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_get.c#6 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_set.c#5 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_strip.c#2 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_support.c#4 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_support.h#5 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_to_text.c#4 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_to_text_nfs4.c#4 edit .. //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_valid.c#3 edit Differences ... ==== //depot/projects/soc2008/trasz_nfs4acl/TODO#16 (text+ko) ==== @@ -48,9 +48,6 @@ - Make 'struct acl' variable size. -- Think about how to make libc implementation - branding, in particular - extensible, - to allow for adding new ACL types. - - Benchmark things. - Add a flag to inode to mark whether the file has ACL; don't try to read ACL extatrr ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_branding.c#4 (text+ko) ==== @@ -54,149 +54,107 @@ } static void -_acl_check_entry(acl_entry_t entry) +_acl_check_entry(const acl_entry_t entry) { assert(entry); assert(entry2acl(entry)->ats_acl.acl_magic == ACL_MAGIC); } +/* + * Return brand of an ACL. + */ int -_acl_is_nfs4(const acl_t acl) +_acl_brand(const acl_t acl) { - if (acl->ats_acl.acl_brand == ACL_BRAND_NFS4) - return (1); - - return (0); + return (acl->ats_acl.acl_brand); } int -_acl_is_posix(const acl_t acl) +_entry_brand(const acl_entry_t entry) { - if (acl->ats_acl.acl_brand == ACL_BRAND_POSIX) - return (1); + _acl_check_entry(entry); - return (0); + return (_acl_brand(entry2acl(entry))); } +/* + * Return 1, iff branding ACL as "brand" is ok. + */ int -_acl_is_unknown(const acl_t acl) +_acl_brand_may_be(const acl_t acl, int brand) { - if (acl->ats_acl.acl_brand == ACL_BRAND_UNKNOWN) + if (_acl_brand(acl) == ACL_BRAND_UNKNOWN) return (1); - return (0); -} - -int -_entry_is_nfs4(const acl_entry_t entry) -{ - _acl_check_entry(entry); - - if (_acl_is_nfs4(entry2acl(entry))) + if (_acl_brand(acl) == brand) return (1); return (0); } int -_entry_is_posix(const acl_entry_t entry) +_entry_brand_may_be(const acl_entry_t entry, int brand) { _acl_check_entry(entry); - if (_acl_is_posix(entry2acl(entry))) - return (1); - - return (0); + return (_acl_brand_may_be(entry2acl(entry), brand)); } -int -_entry_is_unknown(const acl_entry_t entry) +/* + * Brand ACL as "brand". + */ +void +_acl_brand_as(acl_t acl, int brand) { - _acl_check_entry(entry); + assert(_acl_brand_may_be(acl, brand)); - if (_acl_is_unknown(entry2acl(entry))) - return (1); - - return (0); + acl->ats_acl.acl_brand = brand; } -int -_acl_must_be_posix(acl_t acl) +void +_entry_brand_as(const acl_entry_t entry, int brand) { - int type; + _acl_check_entry(entry); - type = acl->ats_acl.acl_brand; - - if (type == ACL_BRAND_POSIX) { - assert(_entry_is_posix(&(acl->ats_acl.acl_entry[3]))); - return (1); - } - - if (type == ACL_BRAND_UNKNOWN) { - acl->ats_acl.acl_brand = ACL_BRAND_POSIX; - assert(_entry_is_posix(&(acl->ats_acl.acl_entry[3]))); - return (1); - } - - return (0); + _acl_brand_as(entry2acl(entry), brand); } int -_acl_must_be_nfs4(acl_t acl) +_acl_type_not_valid_for_acl(const acl_t acl, acl_type_t type) { - int type; + switch (_acl_brand(acl)) { + case ACL_BRAND_NFS4: + if (type == ACL_TYPE_NFS4) + return (0); - type = acl->ats_acl.acl_brand; + break; - if (type == ACL_BRAND_NFS4) { - assert(_entry_is_nfs4(&(acl->ats_acl.acl_entry[3]))); - return (1); - } + case ACL_BRAND_POSIX: + if (type == ACL_TYPE_ACCESS || type == ACL_TYPE_DEFAULT) + return (0); - if (type == ACL_BRAND_UNKNOWN) { - acl->ats_acl.acl_brand = ACL_BRAND_NFS4; - return (1); + break; } - return (0); -} - -int -_entry_must_be_posix(acl_entry_t entry) -{ - _acl_check_entry(entry); - - return (_acl_must_be_posix(entry2acl(entry))); -} - -int -_entry_must_be_nfs4(acl_entry_t entry) -{ - _acl_check_entry(entry); - - return (_acl_must_be_nfs4(entry2acl(entry))); -} - -int -_acl_type_not_valid_for_acl(const acl_t acl, acl_type_t type) -{ - if (_acl_is_nfs4(acl) && type == ACL_TYPE_NFS4) - return (0); - - if (_acl_is_posix(acl) && - (type == ACL_TYPE_ACCESS || type == ACL_TYPE_DEFAULT)) - return (0); - return (-1); } void _acl_brand_from_type(acl_t acl, acl_type_t type) { - if (type == ACL_TYPE_NFS4) { - _acl_must_be_nfs4(acl); - } else { - _acl_must_be_posix(acl); + switch (type) { + case ACL_TYPE_NFS4: + _acl_brand_as(acl, ACL_BRAND_NFS4); + break; + + case ACL_TYPE_ACCESS: + case ACL_TYPE_DEFAULT: + _acl_brand_as(acl, ACL_BRAND_POSIX); + break; + + default: + /* XXX: What to do here? */ + break; } } ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_calc_mask.c#3 (text+ko) ==== @@ -50,11 +50,13 @@ acl_t acl_new; int i, mask_mode, mask_num; - if (!_acl_must_be_posix(*acl_p)) { + if (!_acl_brand_may_be(*acl_p, ACL_BRAND_POSIX)) { errno = EINVAL; return (-1); } + _acl_brand_as(*acl_p, ACL_BRAND_POSIX); + /* * (23.4.2.4) requires acl_p to point to a pointer to a valid ACL. * Since one of the primary reasons to use this function would be ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_copy.c#3 (text+ko) ==== @@ -50,19 +50,15 @@ return (-1); } - if (_entry_is_posix(src_d)) { - if (!_entry_must_be_posix(dest_d)) { - errno = EINVAL; - return (-1); - } + /* + * Can we brand the new entry the same as the source entry? + */ + if (!_entry_brand_may_be(dest_d, _entry_brand(src_d))) { + errno = EINVAL; + return (-1); } - if (_entry_is_nfs4(src_d)) { - if (!_entry_must_be_nfs4(dest_d)) { - errno = EINVAL; - return (-1); - } - } + _entry_brand_as(dest_d, _entry_brand(src_d)); dest_d->ae_tag = src_d->ae_tag; dest_d->ae_id = src_d->ae_id; ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_delete_entry.c#5 (text+ko) ==== @@ -48,7 +48,8 @@ * * XXX: The proper way would be to remove them by entry number. */ - if (_entry_is_nfs4(a)) { + switch (_entry_brand(a)) { + case ACL_BRAND_NFS4: if (a->ae_tag != b->ae_tag || a->ae_extended != b->ae_extended) return (0); @@ -60,10 +61,10 @@ return (1); - } else { + default: if ((a->ae_tag == b->ae_tag) && (a->ae_id == b->ae_id)) return (1); - } + } return (0); } @@ -85,7 +86,7 @@ acl_int = &acl->ats_acl; - if (_entry_is_nfs4(entry_d) != _acl_is_nfs4(acl)) { + if (_entry_brand(entry_d) != _acl_brand(acl)) { errno = EINVAL; return (-1); } ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_flag.c#2 (text+ko) ==== @@ -111,7 +111,7 @@ return (-1); } - if (!_entry_is_nfs4(entry_d)) { + if (!_entry_brand_may_be(entry_d, ACL_BRAND_NFS4)) { errno = EINVAL; return (-1); } @@ -129,11 +129,13 @@ return (-1); } - if (!_entry_must_be_nfs4(entry_d)) { + if (!_entry_brand_may_be(entry_d, ACL_BRAND_NFS4)) { errno = EINVAL; return (-1); } + _entry_brand_as(entry_d, ACL_BRAND_NFS4); + if (_flag_is_invalid(*flagset_d)) return (-1); ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_from_text.c#4 (text+ko) ==== @@ -88,7 +88,7 @@ uid_t id; int error; - assert(_acl_is_posix(aclp)); + assert(_acl_brand(aclp) == ACL_BRAND_POSIX); /* Split into three ':' delimited fields. */ tag = strsep(&entry, ":"); @@ -222,15 +222,26 @@ if (strlen(string_skip_whitespace(entry)) == 0) continue; - if (_acl_is_unknown(acl) && _text_is_nfs4_entry(entry)) - _acl_must_be_nfs4(acl); - else - _acl_must_be_posix(acl); + if (_acl_brand(acl) == ACL_BRAND_UNKNOWN) { + if (_text_is_nfs4_entry(entry)) + _acl_brand_as(acl, ACL_BRAND_NFS4); + else + _acl_brand_as(acl, ACL_BRAND_POSIX); + } - if (_acl_is_nfs4(acl)) + switch (_acl_brand(acl)) { + case ACL_BRAND_NFS4: error = _nfs4_acl_entry_from_text(acl, entry); - else + break; + + case ACL_BRAND_POSIX: error = _posix1e_acl_entry_from_text(acl, entry); + break; + + default: + error = EINVAL; + break; + } if (error) goto error_label; ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_from_text_nfs4.c#3 (text+ko) ==== @@ -205,7 +205,7 @@ if (error) return (error); - assert(_entry_is_nfs4(entry)); + assert(_entry_brand(entry) == ACL_BRAND_NFS4); if (str == NULL) goto truncated_entry; ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_get.c#6 (text+ko) ==== @@ -230,7 +230,7 @@ return (-1); } - if (!_entry_is_nfs4(entry_d)) { + if (!_entry_brand_may_be(entry_d, ACL_BRAND_NFS4)) { errno = EINVAL; return (-1); } ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_set.c#5 (text+ko) ==== @@ -170,10 +170,12 @@ return (-1); } - if (!_entry_must_be_nfs4(entry_d)) { + if (!_entry_brand_may_be(entry_d, ACL_BRAND_NFS4)) { errno = EINVAL; return (-1); } + + _entry_brand_as(entry_d, ACL_BRAND_NFS4); } entry_d->ae_perm = *permset_d; @@ -219,13 +221,25 @@ return (-1); } - if ((tag_type == ACL_OTHER || tag_type == ACL_MASK) && - !_entry_must_be_posix(entry_d)) { - errno = EINVAL; - return (-1); - } else if (tag_type == ACL_EVERYONE && !_entry_must_be_nfs4(entry_d)) { - errno = EINVAL; - return (-1); + switch(tag_type) { + case ACL_OTHER: + case ACL_MASK: + if (!_entry_brand_may_be(entry_d, ACL_BRAND_POSIX)) { + errno = EINVAL; + return (-1); + } + + _entry_brand_as(entry_d, ACL_BRAND_POSIX); + break; + + case ACL_EVERYONE: + if (!_entry_brand_may_be(entry_d, ACL_BRAND_NFS4)) { + errno = EINVAL; + return (-1); + } + + _entry_brand_as(entry_d, ACL_BRAND_NFS4); + break; } switch(tag_type) { @@ -252,11 +266,13 @@ return (-1); } - if (!_entry_must_be_nfs4(entry_d)) { + if (!_entry_brand_may_be(entry_d, ACL_BRAND_NFS4)) { errno = EINVAL; return (-1); } + _entry_brand_as(entry_d, ACL_BRAND_NFS4); + switch (extended) { case ACL_EXTENDED_ALLOW: case ACL_EXTENDED_DENY: ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_strip.c#2 (text+ko) ==== @@ -50,10 +50,7 @@ return (NULL); } - if (_acl_is_nfs4(aclp)) - _acl_must_be_nfs4(newacl); - else - _acl_must_be_posix(newacl); + _acl_brand_as(newacl, ACL_BRAND_NFS4); acl_nfs4_sync_mode_from_acl(&mode, &(aclp->ats_acl)); error = acl_nfs4_sync_acl_from_mode(&(newacl->ats_acl), mode, -1); @@ -74,13 +71,13 @@ acl_tag_t tag; int entry_id, have_mask_entry; - assert(_acl_is_posix(aclp)); + assert(_acl_brand(aclp) == ACL_BRAND_POSIX); acl_old = acl_dup(aclp); if (acl_old == NULL) err(1, "acl_dup() failed"); - assert(_acl_is_posix(aclp)); + assert(_acl_brand(acl_old) == ACL_BRAND_POSIX); have_mask_entry = 0; acl_new = acl_init(ACL_MAX_ENTRIES); @@ -93,7 +90,7 @@ while (acl_get_entry(acl_old, entry_id, &entry) == 1) { entry_id = ACL_NEXT_ENTRY; - assert(_entry_is_posix(entry)); + assert(_entry_brand(entry) == ACL_BRAND_POSIX); if (acl_get_tag_type(entry, &tag) == -1) err(1, "acl_get_tag_type() failed"); @@ -114,7 +111,7 @@ err(1, "acl_get_permset() failed"); if (acl_copy_entry(entry_new, entry) == -1) err(1, "acl_copy_entry() failed"); - assert(_entry_is_posix(entry_new)); + assert(_entry_brand(entry_new) == ACL_BRAND_POSIX); break; case ACL_MASK: have_mask_entry = 1; @@ -124,7 +121,7 @@ } } - assert(_acl_is_posix(acl_new)); + assert(_acl_brand(acl_new) == ACL_BRAND_POSIX); if (have_mask_entry && recalculate_mask) { if (acl_calc_mask(&acl_new) == -1) @@ -137,10 +134,17 @@ acl_t acl_strip_np(const acl_t aclp, int recalculate_mask) { - if (_acl_is_nfs4(aclp)) + switch (_acl_brand(aclp)) { + case ACL_BRAND_NFS4: return (_nfs4_acl_strip_np(aclp, recalculate_mask)); - return (_posix1e_acl_strip_np(aclp, recalculate_mask)); + case ACL_BRAND_POSIX: + return (_posix1e_acl_strip_np(aclp, recalculate_mask)); + + default: + errno = EINVAL; + return (NULL); + } } /* @@ -154,28 +158,36 @@ acl_is_trivial_np(acl_t aclp) { acl_t tmpacl; + int differs; - if (_acl_is_posix(aclp)) { + switch (_acl_brand(aclp)) { + case ACL_BRAND_POSIX: if (aclp->ats_acl.acl_cnt == 3) return (1); return (0); - } + + case ACL_BRAND_NFS4: + /* + * Calculate trivial ACL - using acl_strip_np - and compare + * with the original. + */ + tmpacl = acl_strip_np(aclp, 0); + /* XXX: This sucks. Can this happen at all? */ + if (tmpacl == NULL) + return (0); - /* - * Calculate trivial ACL - using acl_strip_np - and compare - * with the original. - */ - tmpacl = acl_strip_np(aclp, 0); - /* XXX: This sucks. Can this happen at all? */ - if (tmpacl == NULL) - return (0); + differs = _acl_differs(aclp, tmpacl); + acl_free(tmpacl); - if (_acl_differs(aclp, tmpacl)) - return (0); + if (differs) + return (0); - acl_free(tmpacl); + return (1); - return (1); + default: + errno = EINVAL; + return (0); + } } ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_support.c#4 (text+ko) ==== @@ -77,6 +77,10 @@ int i; struct acl_entry *entrya, *entryb; + assert(_acl_brand(a) == _acl_brand(b)); + assert(_acl_brand(a) != ACL_BRAND_UNKNOWN); + assert(_acl_brand(b) != ACL_BRAND_UNKNOWN); + if (a->ats_acl.acl_cnt != b->ats_acl.acl_cnt) return (1); @@ -106,8 +110,8 @@ static int _posix1e_acl_entry_compare(struct acl_entry *a, struct acl_entry *b) { - assert(_entry_is_posix(a)); - assert(_entry_is_posix(b)); + assert(_entry_brand(a) == ACL_BRAND_POSIX); + assert(_entry_brand(b) == ACL_BRAND_POSIX); /* * First, sort between tags -- conveniently defined in the correct @@ -152,7 +156,7 @@ acl_int = &acl->ats_acl; /* XXX: */ - assert(_entry_is_posix(&(acl->ats_acl.acl_entry[3]))); + assert(_entry_brand(&(acl->ats_acl.acl_entry[3])) == ACL_BRAND_POSIX); qsort(&acl_int->acl_entry[0], acl_int->acl_cnt, sizeof(struct acl_entry), (compare) _posix1e_acl_entry_compare); @@ -168,7 +172,7 @@ int _posix1e_acl(acl_t acl, acl_type_t type) { - if (!_acl_is_posix(acl)) + if (_acl_brand(acl) != ACL_BRAND_POSIX) return (0); return ((type == ACL_TYPE_ACCESS) || (type == ACL_TYPE_DEFAULT)); ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_support.h#5 (text+ko) ==== @@ -37,18 +37,14 @@ int _acl_type_unold(acl_type_t type); int _acl_differs(const acl_t a, const acl_t b); -int _acl_is_nfs4(const acl_t acl); -int _acl_is_posix(const acl_t acl); -int _acl_is_unknown(const acl_t acl); -int _entry_is_nfs4(const acl_entry_t entry); -int _entry_is_posix(const acl_entry_t entry); -int _entry_is_unknown(const acl_entry_t entry); -int _acl_must_be_posix(acl_t acl); -int _acl_must_be_nfs4(acl_t acl); -int _entry_must_be_posix(acl_entry_t entry); -int _entry_must_be_nfs4(acl_entry_t entry); int _acl_type_not_valid_for_acl(const acl_t acl, acl_type_t type); void _acl_brand_from_type(acl_t acl, acl_type_t type); +int _acl_brand(const acl_t acl); +int _entry_brand(const acl_entry_t entry); +int _acl_brand_may_be(const acl_t acl, int brand); +int _entry_brand_may_be(const acl_entry_t entry, int brand); +void _acl_brand_as(acl_t acl, int brand); +void _entry_brand_as(const acl_entry_t entry, int brand); int _posix1e_acl_check(acl_t acl); int _posix1e_acl_sort(acl_t acl); int _posix1e_acl(acl_t acl, acl_type_t type); ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_to_text.c#4 (text+ko) ==== @@ -242,10 +242,17 @@ char * acl_to_text_np(acl_t acl, ssize_t *len_p, int verbose) { - if (_acl_is_nfs4(acl)) + switch (_acl_brand(acl)) { + case ACL_BRAND_POSIX: + return (_posix1e_acl_to_text(acl, len_p)); + + case ACL_BRAND_NFS4: return (_nfs4_acl_to_text_np(acl, len_p, verbose)); - return (_posix1e_acl_to_text(acl, len_p)); + default: + errno = EINVAL; + return (NULL); + } } char * ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_to_text_nfs4.c#4 (text+ko) ==== @@ -138,7 +138,7 @@ acl_permset_t permset; acl_flagset_t flagset; - assert(_entry_is_nfs4(entry)); + assert(_entry_brand(entry) == ACL_BRAND_NFS4); if (acl_get_flagset_np(entry, &flagset)) return (0); ==== //depot/projects/soc2008/trasz_nfs4acl/lib/libc/posix1e/acl_valid.c#3 (text+ko) ==== @@ -63,7 +63,7 @@ return (-1); } - if (_acl_is_nfs4(acl)) { + if (!_acl_brand_may_be(acl, ACL_BRAND_POSIX)) { errno = EINVAL; return (-1); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200807281338.m6SDcsuN013197>