From owner-freebsd-current@FreeBSD.ORG Wed Feb 9 21:10:44 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A3F91106566C for ; Wed, 9 Feb 2011 21:10:44 +0000 (UTC) (envelope-from lattera@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 2B9F28FC17 for ; Wed, 9 Feb 2011 21:10:43 +0000 (UTC) Received: by wyf19 with SMTP id 19so633606wyf.13 for ; Wed, 09 Feb 2011 13:10:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=NPdcELT3RlM1Bp01hv9LlLViES8/rZOOcMpBhQ/Kie8=; b=b356ljf+geWA4gVoHVu2+TVuGCVHdiG/GKWamj3TnCRks6ggcG+PgbPYx4rizw+PpM O4gi0GBuBTWvQb7GdaapKdNwh5M9+PP/CGN8wWUqOCE/M2RFTInPvV+MD/yOCCKU4DxZ FsGGYTp5aeNbAPriJynjkVPRxBV2046mYMuBo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=URJLkBfIimq+ZxZOTMsrQi4C9sPoieb0ihZUdHNl4HASr/YlwoyiZK5Z4nBBOqD/EC zX6TAt4Hl8klKIKdxKfq+ZI1Em3gt5h/3nKTdWD/K9Vk9LUrS2yEFGx+T9MLSXuDQ7jE vlsn5oSLEPNghRbwNKwlKPCf44uCD18oJb8gc= MIME-Version: 1.0 Received: by 10.227.41.204 with SMTP id p12mr13491499wbe.153.1297285842982; Wed, 09 Feb 2011 13:10:42 -0800 (PST) Received: by 10.227.68.204 with HTTP; Wed, 9 Feb 2011 13:10:42 -0800 (PST) In-Reply-To: References: <80373F51-25C7-48A0-8920-3444A98D857F@kientzle.com> Date: Wed, 9 Feb 2011 14:10:42 -0700 Message-ID: From: Shawn Webb To: Tim Kientzle Content-Type: multipart/mixed; boundary=002215974e76d7d792049bdfe548 X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: FreeBSD-current Subject: Re: setfacl Recursive Functionality X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Feb 2011 21:10:44 -0000 --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::read_data:file_inherit/dir_inherit:allow ` and not run into errors. Thanks, Shawn On Tue, Feb 8, 2011 at 7:51 PM, Shawn Webb wrote: > On Tue, Feb 8, 2011 at 7:35 PM, Tim Kientzle 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--