Hi Serge, On Tue, Apr 15, 2014 at 10:57:52AM -0500, Serge Hallyn wrote: > Quoting Aristeu Rozanski (aris@xxxxxxxxxx): > > may_access() is currently used to validate both new exceptions from children > > groups and to check if an access is allowed. This not only makes it hard to > > understand and maintain, but it's also incorrect. > > Heh, well one might argue that may_access() was more approriate at > __devcgroup_check_permission(), should remain simpler, and parent_has_perm() > should be the wrapper doing the extra bits. That would be easier to > read imo. > > In what you have here, you end up duplicating the > list_for_each_entry_rcu. I think you should at least have a common > fn for that. > > I don't want to sound pedantic, but since the point of this patch is > that simplicity/ease-of-reading would have prevented the bug ... :) Yes, trying to get this right this time. The problem itself isn't simple, comparing ranges when adding new rules on the child and the possible difference between the meaning of the exception (is it to disallow access in that range or to allow?) makes things complicated. > I *think* it's ok, and I appreciate you finding and fixing the bug, but > I don't think the result is as easy to read as it could be, so if you > don't mind I'd like to wait for a new version to ack. If you disagree > with me, let me know and I'll re-read and (presumably) ack. Yes, don't worry about it, I'm working on a v2 that will makes things easier to read. If not, I'll keep working on it until it looks simple enough. -- Aristeu -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
