Re: Comments to apparmor Makefile (and security/Makefile)

[John Johansen]

On 08/28/2010 12:03 AM, Sam Ravnborg wrote:
> Hi John.
> 
> I took a closer at security/apparmor/Makefile.
> 
> And I got a few comments...
> 
> 1) You have in the bottom explicit rules for three files:
>    capability_names.h, rlim_names.h and af_names.h
>    But the altter file is not referenced in any
>    apparmor file.
>    And there is no cmd_make-af variable defined.
>    Looks like a leftover that shall be dropped.
> 
yes, the network mediation was split out for later submission and
af_names.h should have been removed.

> 2) clean-files fails to include rlim_names.h
> 
fixed.

> 3) The cmd_make-caps line is much too long.
>    Please use "\" to break lines in smaller logical parts.
>    Same goes with the other cdm_ line.
> 
ok

> 4) make-rlim delete the symbol AF_MAX - but that does not
>   exist in the input file.
> 
yep

> 5) Two of the sed expressions looks almost equal - should they have been equal?
> 
no, they actually produce slightly different output.

> 6) A small comment describing the purpose of each sed expression would be helpfull
>    Something like:
> # Transform lines from:
> # #define FOO 123
> # =>
> # #define RLIM_FOO 123
> 
yeah that would be good to have

> 7) af_names.h needs to be dropped in .gitignore
> 
yep, A patch for this was just floated by Yong Zhang

> 8) From security/Makefile:
> 
>     obj-$(CONFIG_SECURITY_APPARMOR)         += apparmor/built-in.o
> This is _not_ how we do it.
> 
> We say just:
> 
>     obj-$(CONFIG_SECURITY_APPARMOR)         += apparmor/
> 
> [security/Makefile has this issue in several places]
> 
okay, I set it up this way to conform to other entries in the file
If we are going to fix apparmor's entry we should fix them all

> There was a few tings that made me unsafe just providing a patch,
> so for now you got a list of comments.
> 
> 	Sam

np, thanks for the comments, patch attached

john

---