[apparmor] [patch 2/6] abstract out cap and net proto generation to common/Make.rules

[Christian Boltz]

Hello,

(also affects patch 3/6, but splitting the mail wouldn't make sense)

Am Donnerstag, 22. März 2012 schrieb Steve Beattie:
> This patch abstracts out the generation of the lists of capabilities
> and network protocol names to the common Make.rules file that is
> included in most locations in the build tree, to allow it to be
> re-used in the utils/ tree and possibly elsewhere.

I like the idea, but the implementation is, well, suboptimal...

> --- a/common/Make.rules
> +++ b/common/Make.rules
> @@ -151,6 +151,40 @@ _clean:
>  	-rm -f ${MANPAGES} *.[0-9].gz ${HTMLMANPAGES} pod2htm*.tmp
> 
>  # =====================
> +# generate list of capabilities based on
> +# /usr/include/sys/capabilities.h for use in multiple locations in

I don't have /usr/include/sys/capability.h on my system (openSUSE 12.1)
Either I need to install another package (thanks to OBS, I don't have 
too many devel packages on my system) or it's at another location.

# locate capability.h
/usr/include/linux/capability.h
/usr/src/linux-3.1.0-1.2/include/linux/capability.h
/usr/src/linux-3.1.9-1.4/include/linux/capability.h

Do I miss a package or are the paths really different on openSUSE?

> +# emits defined capabilities in a simple list, e.g. "CAP_NAME
> CAP_NAME2" 
> +CAPABILITIES=$(shell echo "\#include <sys/capability.h>" | cpp -dM | 
LC_ALL=C sed -n -e '/CAP_EMPTY_SET/d' -e 's/^\#define[ \t]\+CAP_\([A-
Z0-9_]\+\)[ \t]\+\([0-9xa-f]\+\)\(.*\)$$/CAP_\1/p' | sort) 

Now let me paste a sniplet from patch 3/6 (utils/Makefile):
> +# ${CAPABILITIES} is defined in common/Make.rules
> +.PHONY: check_severity_db
> +.SILENT: check_severity_db
> +check_severity_db: /usr/include/sys/capability.h severity.db

The problem I see here is that the Makefile contains an "indirect" 
dependency. IMHO that's not a clean solution and might cause maintenance
fun if capability.h ever moves.


I'd like to propose an alternative solution that avoids this problem:

In common/Make.rules, write the capability list to a file instead of 
storing it in a variable:

capability_list: /usr/include/linux/capability.h
    echo "\#include <sys/capability.h>" | cpp -dM | LC_ALL=C sed -n -e 
'/CAP_EMPTY_SET/d' -e 's/^\#define[ \t]\+CAP_\([A-Z0-9_]\+\)[ 
\t]\+\([0-9xa-f]\+\)\(.*\)$$/CAP_\1/p' | sort > capability_list


now back to utils/Makefile:
> +check_severity_db: /usr/include/sys/capability.h severity.db
> +       # The sed statement is based on the one in the parser's 
makefile

Outdated comment? I see no sed in the check_severity_db target.

> +       RC=0 ; for cap in ${CAPABILITIES} ; do \

Would then be

check_severity_db: capability_list severity.db
     RC=0 ; for cap in `cat capability_list` ; do \


AF_NAMES shares this problem and should also be implemented with a file
instead of using a make variable.

Note that everything above is untested ;-)

BTW: "make clean" should delete the capability_list file.


Regards,

Christian Boltz
-- 
Unix: Alles ist ein File, und was kein File ist, hat sich gefaelligst
als ein solches zu tarnen.      [Wolfgang Weisselberg in linux-liste]