Date: Wed, 9 Feb 2011 14:10:42 -0700 From: Shawn Webb <lattera@gmail.com> To: Tim Kientzle <tim@kientzle.com> Cc: FreeBSD-current <freebsd-current@freebsd.org> Subject: Re: setfacl Recursive Functionality Message-ID: <AANLkTim9anqAqSLH=5OMi_dTyAsqY2xCL37KM8MmrZdR@mail.gmail.com> In-Reply-To: <AANLkTin2XR9Ra4LK-rgh94XaAUB6jY_sg0n51GdbgnQ2@mail.gmail.com> References: <AANLkTi=%2BWtmRz07m=Cg7hbXJGw7eWRHC1ASGeufTSLBB@mail.gmail.com> <80373F51-25C7-48A0-8920-3444A98D857F@kientzle.com> <AANLkTin2XR9Ra4LK-rgh94XaAUB6jY_sg0n51GdbgnQ2@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--002215974e76d7d792049bdfe548 Content-Type: text/plain; charset=ISO-8859-1 Included in the attached patch is the refactor using fts(3) and with the -L and -H options. I'm still looking for feedback and suggestions on how to improve the patch. I'll also port these changes over to my getfacl patch. If anyone's interested in following up-to-date development of the patch, the link to it on github is https://github.com/lattera/patches/blob/master/freebsd/setfacl_recursive.patch I'd like to take the time to address why I created the remove_invalid_inherit function since I got a private email asking why it existed. Other than symbolic links, non-directory entries cannot have inheritance set. That function prevents attempting to set inheritance flags on non-directory entries when doing a recursive call. That way, you can run `setfacl -R -m user:<user>:read_data:file_inherit/dir_inherit:allow <directory>` and not run into errors. Thanks, Shawn On Tue, Feb 8, 2011 at 7:51 PM, Shawn Webb <lattera@gmail.com> wrote: > On Tue, Feb 8, 2011 at 7:35 PM, Tim Kientzle <tim@kientzle.com> wrote: > >> On Feb 8, 2011, at 9:58 AM, Shawn Webb wrote: >> > I've just finished a patch to add recursive functionality to setfacl. >> Before >> > I officially submit it, I'd like a few suggestions on how to improve the >> > patch. >> > >> > The part I'm worried about involves the #define directive at top. I'm >> not >> > sure what ramifications using that define might have. I needed it for my >> > remove_invalid_inherit() function to work. >> >> You should certainly not need >> #define _ACL_PRIVATE >> for any user-space utilities. What exactly is the >> problem without that? >> >> Your approach to directory walking here >> is a little simplistic. In particular, you're storing >> every filename for the entire tree in memory, >> which is a problem for large filesystems. >> >> It would be much better to refactor the code so that >> the actual ACL update was in a function and then >> recurse_directory should call that function for >> each filename as it visited it. That will reduce >> the memory requirements significantly. >> >> You should also take a look at fts(3). In particular, >> you'll want to implement the BSD-standard >> -L/-P/-H options, and fts(3) makes that much easier. >> (-L always follows symlinks, -P never follows symlinks, >> -H follows symlinks on the command line). >> >> Tim >> >> > Great suggestions. I'll definitely look at implementing that functionality. > > As a side note, it looks like my setfacl patch segfaults on freebsd-current > r218075 with the zpool v28 patchset applied. I wrote it on freebsd 8.2-RC3 > with zpool v15. > > Thanks, > > Shawn > --002215974e76d7d792049bdfe548 Content-Type: application/octet-stream; name="setfacl_recursive.patch" Content-Disposition: attachment; filename="setfacl_recursive.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_gjyqcwxv0 LS0tIC91c3Ivc3JjL2Jpbi9zZXRmYWNsL3NldGZhY2wuYwkyMDExLTAyLTAzIDEyOjExOjAyLjMw MzQ5NjMxOCAtMDcwMAorKysgYmluL3NldGZhY2wvc2V0ZmFjbC5jCTIwMTEtMDItMDkgMTM6NTk6 MDYuOTYxNTM4MDU2IC0wNzAwCkBAIC0yMyw3ICsyMyw3IEBACiAgKiBBUklTSU5HIElOIEFOWSBX QVkgT1VUIE9GIFRIRSBVU0UgT0YgVEhJUyBTT0ZUV0FSRSwgRVZFTiBJRiBBRFZJU0VEIE9GIFRI RQogICogUE9TU0lCSUxJVFkgT0YgU1VDSCBEQU1BR0UuCiAgKi8KLQorI2RlZmluZSBfQUNMX1BS SVZBVEUKICNpbmNsdWRlIDxzeXMvY2RlZnMuaD4KIF9fRkJTRElEKCIkRnJlZUJTRCQiKTsKIApA QCAtMzIsNiArMzIsOCBAQAogI2luY2x1ZGUgPHN5cy9zdGF0Lmg+CiAjaW5jbHVkZSA8c3lzL2Fj bC5oPgogI2luY2x1ZGUgPHN5cy9xdWV1ZS5oPgorI2luY2x1ZGUgPGZ0cy5oPgorI2luY2x1ZGUg PGRpcmVudC5oPgogCiAjaW5jbHVkZSA8ZXJyLmg+CiAjaW5jbHVkZSA8ZXJybm8uaD4KQEAgLTQ0 LDYgKzQ2LDggQEAKIAogc3RhdGljIHZvaWQJYWRkX2ZpbGVuYW1lKGNvbnN0IGNoYXIgKmZpbGVu YW1lKTsKIHN0YXRpYyB2b2lkCXVzYWdlKHZvaWQpOworc3RhdGljIHZvaWQJcmVjdXJzZV9kaXJl Y3RvcnkoY2hhciAqY29uc3QgKnBhdGhzLCBpbnQgcl9mbGFnLCBpbnQgbF9mbGFnLCBpbnQgYmln X2hfZmxhZyk7CitzdGF0aWMgYWNsX3QJcmVtb3ZlX2ludmFsaWRfaW5oZXJpdChjb25zdCBjaGFy ICpwYXRoLCBhY2xfdCBhY2wsIGludCBsX2ZsYWcpOwogCiBzdGF0aWMgdm9pZAogYWRkX2ZpbGVu YW1lKGNvbnN0IGNoYXIgKmZpbGVuYW1lKQpAQCAtNjIsMzQgKzY2LDEwNiBAQAogc3RhdGljIHZv aWQKIHVzYWdlKHZvaWQpCiB7Ci0KLQlmcHJpbnRmKHN0ZGVyciwgInVzYWdlOiBzZXRmYWNsIFst YmRoa25dIFstYSBwb3NpdGlvbiBlbnRyaWVzXSAiCisJZnByaW50ZihzdGRlcnIsICJ1c2FnZTog c2V0ZmFjbCBbLWJkaGtMblJdIFstYSBwb3NpdGlvbiBlbnRyaWVzXSAiCiAJICAgICJbLW0gZW50 cmllc10gWy1NIGZpbGVdIFsteCBlbnRyaWVzXSBbLVggZmlsZV0gW2ZpbGUgLi4uXVxuIik7CiAJ ZXhpdCgxKTsKIH0KIAorc3RhdGljIHZvaWQKK3JlY3Vyc2VfZGlyZWN0b3J5KGNoYXIgKmNvbnN0 ICpwYXRocywgaW50IHJfZmxhZywgaW50IGxfZmxhZywgaW50IGJpZ19oX2ZsYWcpCit7CisJRlRT ICpmdHNwOworCUZUU0VOVCAqcCwgKmNocDsKKwlpbnQgZnRzX29wdGlvbnMgPSBGVFNfTk9DSERJ UjsKKwl1bnNpZ25lZCBpbnQgaTsKKwkKKwlmdHNfb3B0aW9ucyB8PSAobF9mbGFnID09IDEpID8g RlRTX0xPR0lDQUwgOiBGVFNfUEhZU0lDQUw7CisJaWYgKGJpZ19oX2ZsYWcpCisJCWZ0c19vcHRp b25zIHw9IEZUU19DT01GT0xMT1c7CisJCisJaWYgKHJfZmxhZykKKwl7CisJCWZ0c3AgPSBmdHNf b3BlbihwYXRocywgZnRzX29wdGlvbnMsIE5VTEwpOworCQlpZiAoZnRzcCA9PSBOVUxMKQorCQkJ cmV0dXJuOworCQkKKwkJY2hwID0gZnRzX2NoaWxkcmVuKGZ0c3AsIDApOworCQlpZiAoY2hwID09 IE5VTEwpCisJCQlyZXR1cm47CisJCQorCQl3aGlsZSAoKHAgPSBmdHNfcmVhZChmdHNwKSkgIT0g TlVMTCkgeworCQkJaWYgKGxfZmxhZyA9PSAwICYmIHAtPmZ0c19pbmZvICYgRlRTX0QpCisJCQkJ Y29udGludWU7CisJCQllbHNlIGlmIChsX2ZsYWcgPT0gMSAmJiBwLT5mdHNfaW5mbyAmIEZUU19E UCkKKwkJCQljb250aW51ZTsKKwkJCQorCQkJYWRkX2ZpbGVuYW1lKHN0cmR1cChwLT5mdHNfcGF0 aCkpOworCQl9CisJCQorCQlmdHNfY2xvc2UoZnRzcCk7CisJfSBlbHNlCisJCWZvciAoaSA9IDA7 IHBhdGhzW2ldICE9IE5VTEw7IGkrKykKKwkJCWFkZF9maWxlbmFtZShwYXRoc1tpXSk7Cit9CisK K3N0YXRpYyBhY2xfdAorcmVtb3ZlX2ludmFsaWRfaW5oZXJpdChjb25zdCBjaGFyICpwYXRoLCBh Y2xfdCBhY2wsIGludCBsX2ZsYWcpCit7CisJYWNsX3QgYWNsX25ldzsKKwlpbnQgYWNsX2JyYW5k OworCWFjbF9lbnRyeV90IGVudHJ5OworCWludCBlbnRyeV9pZDsKKwlzdHJ1Y3Qgc3RhdCBzYjsK KwkKKwlhY2xfZ2V0X2JyYW5kX25wKGFjbCwgJmFjbF9icmFuZCk7CisJaWYgKGFjbF9icmFuZCAh PSBBQ0xfQlJBTkRfTkZTNCkKKwkJcmV0dXJuIGFjbDsKKwkKKwlpZiAobF9mbGFnID09IDEpIHsK KwkJaWYgKHN0YXQocGF0aCwgJnNiKSA9PSAtMSkKKwkJCXJldHVybiBhY2w7CisJfSBlbHNlCisJ CWlmIChsc3RhdChwYXRoLCAmc2IpID09IC0xKQorCQkJcmV0dXJuIGFjbDsKKwkKKwlpZiAoU19J U0RJUihzYi5zdF9tb2RlKSAhPSAwKQorCQlyZXR1cm4gYWNsOworCQorCWFjbF9uZXcgPSBhY2xf ZHVwKGFjbCk7CisJCisJZW50cnlfaWQgPSBBQ0xfRklSU1RfRU5UUlk7CisJd2hpbGUgKGFjbF9n ZXRfZW50cnkoYWNsX25ldywgZW50cnlfaWQsICZlbnRyeSkgPT0gMSkgeworCQllbnRyeV9pZCA9 IEFDTF9ORVhUX0VOVFJZOworCQllbnRyeS0+YWVfZmxhZ3MgPSAwOworCX0KKwkKKwlyZXR1cm4g YWNsX25ldzsKK30KKwogaW50CiBtYWluKGludCBhcmdjLCBjaGFyICphcmd2W10pCiB7Ci0JYWNs X3QgYWNsOworCWFjbF90IGFjbCwgYWNsX2JhY2t1cDsKIAlhY2xfdHlwZV90IGFjbF90eXBlOwog CWNoYXIgZmlsZW5hbWVbUEFUSF9NQVhdOwotCWludCBsb2NhbF9lcnJvciwgY2FycmllZF9lcnJv ciwgY2gsIGksIGVudHJ5X251bWJlciwgcmV0OwotCWludCBoX2ZsYWc7CisJaW50IGxvY2FsX2Vy cm9yLCBjYXJyaWVkX2Vycm9yLCBjaCwgZW50cnlfbnVtYmVyLCByZXQ7CisJaW50IGhfZmxhZywg cl9mbGFnLCBsX2ZsYWcsIGJpZ19oX2ZsYWc7CiAJc3RydWN0IHNmX2ZpbGUgKmZpbGU7CiAJc3Ry dWN0IHNmX2VudHJ5ICplbnRyeTsKLQljb25zdCBjaGFyICpmbl9kdXA7CisJY2hhciAqZm5fZHVw OwogCWNoYXIgKmVuZDsKKwljaGFyICoqZmlsZXM9TlVMTDsKKwl1bnNpZ25lZCBpbnQgbnVtZmls ZXM9MDsKIAlzdHJ1Y3Qgc3RhdCBzYjsKIAogCWFjbF90eXBlID0gQUNMX1RZUEVfQUNDRVNTOwog CWNhcnJpZWRfZXJyb3IgPSBsb2NhbF9lcnJvciA9IDA7Ci0JaF9mbGFnID0gaGF2ZV9tYXNrID0g aGF2ZV9zdGRpbiA9IG5fZmxhZyA9IG5lZWRfbWFzayA9IDA7CisJaF9mbGFnID0gaGF2ZV9tYXNr ID0gaGF2ZV9zdGRpbiA9IG5fZmxhZyA9IG5lZWRfbWFzayA9IHJfZmxhZyA9IGxfZmxhZyA9IGJp Z19oX2ZsYWcgPSAwOwogCiAJVEFJTFFfSU5JVCgmZW50cnlsaXN0KTsKIAlUQUlMUV9JTklUKCZm aWxlbGlzdCk7CiAKLQl3aGlsZSAoKGNoID0gZ2V0b3B0KGFyZ2MsIGFyZ3YsICJNOlg6YTpiZGhr bTpueDoiKSkgIT0gLTEpCisJd2hpbGUgKChjaCA9IGdldG9wdChhcmdjLCBhcmd2LCAiSExSTTpY OmE6YmRoa206bng6IikpICE9IC0xKQogCQlzd2l0Y2goY2gpIHsKIAkJY2FzZSAnTSc6CiAJCQll bnRyeSA9IHptYWxsb2Moc2l6ZW9mKHN0cnVjdCBzZl9lbnRyeSkpOwpAQCAtMTY3LDYgKzI0Mywx NSBAQAogCQkJfQogCQkJVEFJTFFfSU5TRVJUX1RBSUwoJmVudHJ5bGlzdCwgZW50cnksIG5leHQp OwogCQkJYnJlYWs7CisJCWNhc2UgJ1InOgorCQkJcl9mbGFnID0gMTsKKwkJCWJyZWFrOworCQlj YXNlICdMJzoKKwkJCWxfZmxhZyA9IDE7CisJCQlicmVhazsKKwkJY2FzZSAnSCc6CisJCQliaWdf aF9mbGFnID0gMTsKKwkJCWJyZWFrOwogCQlkZWZhdWx0OgogCQkJdXNhZ2UoKTsKIAkJCWJyZWFr OwpAQCAtMTg5LDExICsyNzQsMTggQEAKIAkJCWZuX2R1cCA9IHN0cmR1cChmaWxlbmFtZSk7CiAJ CQlpZiAoZm5fZHVwID09IE5VTEwpCiAJCQkJZXJyKDEsICJzdHJkdXAoKSBmYWlsZWQiKTsKLQkJ CWFkZF9maWxlbmFtZShmbl9kdXApOworCQkJZmlsZXMgPSByZWFsbG9jKGZpbGVzLCArK251bWZp bGVzICogc2l6ZW9mKGNoYXIgKiopKTsKKwkJCWlmIChmaWxlcyA9PSBOVUxMKQorCQkJCWVycigx LCAicmVhbGxvYygpIGZhaWxlZCIpOworCQkJZmlsZXNbbnVtZmlsZXMtMV0gPSAoY2hhciAqKWZu X2R1cDsKIAkJfQorCQkKKwkJZmlsZXMgPSByZWFsbG9jKGZpbGVzLCArK251bWZpbGVzICogc2l6 ZW9mKGNoYXIgKiopKTsKKwkJZmlsZXNbbnVtZmlsZXMtMV0gPSBOVUxMOwogCX0gZWxzZQotCQlm b3IgKGkgPSAwOyBpIDwgYXJnYzsgaSsrKQotCQkJYWRkX2ZpbGVuYW1lKGFyZ3ZbaV0pOworCQlm aWxlcyA9IGFyZ3Y7CisJCisJcmVjdXJzZV9kaXJlY3RvcnkoZmlsZXMsIHJfZmxhZywgbF9mbGFn LCBiaWdfaF9mbGFnKTsKIAogCS8qIGN5Y2xlIHRocm91Z2ggZWFjaCBmaWxlICovCiAJVEFJTFFf Rk9SRUFDSChmaWxlLCAmZmlsZWxpc3QsIG5leHQpIHsKQEAgLTI1MCwxMiArMzQyLDI0IEBACiAK IAkJCXN3aXRjaChlbnRyeS0+b3ApIHsKIAkJCWNhc2UgT1BfQUREX0FDTDoKKwkJCQlhY2xfYmFj a3VwID0gZW50cnktPmFjbDsKKwkJCQllbnRyeS0+YWNsID0gcmVtb3ZlX2ludmFsaWRfaW5oZXJp dChmaWxlLT5maWxlbmFtZSwgZW50cnktPmFjbCwgbF9mbGFnKTsKIAkJCQlsb2NhbF9lcnJvciAr PSBhZGRfYWNsKGVudHJ5LT5hY2wsCiAJCQkJICAgIGVudHJ5LT5lbnRyeV9udW1iZXIsICZhY2ws IGZpbGUtPmZpbGVuYW1lKTsKKwkJCQlpZiAoZW50cnktPmFjbCAhPSBhY2xfYmFja3VwKSB7CisJ CQkJCWFjbF9mcmVlKGVudHJ5LT5hY2wpOworCQkJCQllbnRyeS0+YWNsID0gYWNsX2JhY2t1cDsK KwkJCQl9CiAJCQkJYnJlYWs7CiAJCQljYXNlIE9QX01FUkdFX0FDTDoKKwkJCQlhY2xfYmFja3Vw ID0gZW50cnktPmFjbDsKKwkJCQllbnRyeS0+YWNsID0gcmVtb3ZlX2ludmFsaWRfaW5oZXJpdChm aWxlLT5maWxlbmFtZSwgZW50cnktPmFjbCwgbF9mbGFnKTsKIAkJCQlsb2NhbF9lcnJvciArPSBt ZXJnZV9hY2woZW50cnktPmFjbCwgJmFjbCwKIAkJCQkgICAgZmlsZS0+ZmlsZW5hbWUpOworCQkJ CWlmIChlbnRyeS0+YWNsICE9IGFjbF9iYWNrdXApIHsKKwkJCQkJYWNsX2ZyZWUoZW50cnktPmFj bCk7CisJCQkJCWVudHJ5LT5hY2wgPSBhY2xfYmFja3VwOworCQkJCX0KIAkJCQluZWVkX21hc2sg PSAxOwogCQkJCWJyZWFrOwogCQkJY2FzZSBPUF9SRU1PVkVfRVhUOgo= --002215974e76d7d792049bdfe548--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTim9anqAqSLH=5OMi_dTyAsqY2xCL37KM8MmrZdR>