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] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor