Re: [TOMOYO #10 (linux-next) 7/8] File operation restriction part.

From: Serge E. Hallyn
Date: Wed Oct 15 2008 - 12:43:25 EST


Quoting Tetsuo Handa (penguin-kernel@xxxxxxxxxxxxxxxxxxx):
> Hello.
>
> Serge E. Hallyn wrote:
> > In a previous patch you mark funtions with 'begin/end critical section'.
> > Please instead put a comment on top listing precisely which locks
> > the fn expects to be held.
> >
> > As for protecting your own data, please
> > 1. explain at the var declaration what lock protects it
> > 2. define the lock next to the list
>
> OK. I added comments and simplified dependencies.
> http://svn.sourceforge.jp/cgi-bin/viewcvs.cgi/trunk/2.2.x/tomoyo-lsm/patches/?root=tomoyo

Cool, thanks.

This, in general, is part of changing your mindset - from that of being
a maintainer of out-of-tree code, to being a part of the core community.
My dcache comment further down is along the same lines.

> Anything else we can do before reposting as #11?

Well I'd like to sit down one day and make sure that your _clean()s in
patch 1 cover all the error cases and there are no leaks.

The pathname walking code doesn't seem to be in any way tomoyo-specific,
so it really ought to be in fs/dcache.c where the relevant maintainers
will see, scrutinize, and update it when necessary. I realize that
means we make it look like we encourage others to use the functions
which we don't want either. But having them in tomoyo-specific code
isn't nice either.

And I haven't really looked at your patches 6-8 yet, and am not sure
when I'll get time.

Anyway I think we're well to the point where the patches should be
tossed into a tree and tested (once you address Paul's feedback).
Actually, one thing which is missing from this patchset is a MAINTAINERS
entry. What I'd particularly be interested in is a mailing list entry
(with a public readable archive), so we can see that there is in fact a
community using this.

-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/