[apparmor] [PATCH] clean up warnings from parser rework

Kees Cook kees at ubuntu.com
Tue Nov 9 21:18:28 GMT 2010


This cleans up a number of warnings that appeared after the parser rework
commits were made (as well as a few other minor warnings elsewhere).

The Makefile change is to avoid passing -Wstrict-prototypes and
-Wnested-externs to the C++ compiler, which the compiler yells about and
then ignores.

Since we compile with -Wmissing-field-initializers I dropped the
unreferenced zero-width fields in the header structs, and then explicitly
initialized the remaining fields.

I tagged several unused function parameters to silence those warnings.

And finally, I dropped the unused filter_escapes() too.

---
 libraries/libapparmor/src/scanner.l |    2 +
 parser/Makefile                     |   12 ++++----
 parser/libapparmor_re/flex-tables.h |    8 ++---
 parser/libapparmor_re/regexp.y      |   16 ++++++-----
 parser/parser_lex.l                 |    1 
 parser/parser_regex.c               |   51 ------------------------------------
 6 files changed, 23 insertions(+), 67 deletions(-)

=== modified file 'libraries/libapparmor/src/scanner.l'
--- libraries/libapparmor/src/scanner.l	2010-09-09 23:51:44 +0000
+++ libraries/libapparmor/src/scanner.l	2010-11-09 20:31:17 +0000
@@ -17,6 +17,8 @@
  */
 
 %option noyywrap
+%option nounput
+%option noyy_top_state
 %option reentrant
 %option prefix="aalogparse_"
 %option bison-bridge

=== modified file 'parser/Makefile'
--- parser/Makefile	2010-11-09 19:33:40 +0000
+++ parser/Makefile	2010-11-09 21:00:36 +0000
@@ -38,13 +38,14 @@
 YFLAGS	:= -d
 LEX	:= /usr/bin/flex
 LEXFLAGS = -B -v
-WARNINGS = -Wall -Wstrict-prototypes
-EXTRA_WARNINGS = -Wsign-compare -Wmissing-field-initializers -Wnested-externs -Wformat-security -Wunused-parameter
-WARNINGS += $(shell for warning in ${EXTRA_WARNINGS} ; do \
+WARNINGS = -Wall
+EXTRA_WARNINGS = -Wsign-compare -Wmissing-field-initializers -Wformat-security -Wunused-parameter
+CXX_WARNINGS = ${WARNINGS} $(shell for warning in ${EXTRA_WARNINGS} ; do \
 			if ${CC} $${warning} -S -o /dev/null -xc /dev/null >/dev/null 2>&1; then \
 				echo "$${warning}"; \
 			fi ; \
 		done)
+CPP_WARNINGS = -Wstrict-prototypes -Wnested-externs
 ifndef CFLAGS
 CFLAGS	= -g -O2 -pipe
 
@@ -53,7 +54,8 @@
 endif
 endif #CFLAGS
 
-EXTRA_CFLAGS = ${CFLAGS} ${WARNINGS} -D_GNU_SOURCE
+EXTRA_CXXFLAGS = ${CFLAGS} ${CXX_WARNINGS} -D_GNU_SOURCE
+EXTRA_CFLAGS = ${EXTRA_CXXFLAGS} ${CPP_WARNINGS}
 
 #LEXLIB	:= -lfl
 
@@ -235,7 +237,7 @@
 .SILENT: $(AAREOBJECTS)
 .PHONY: $(AAREOBJECTS)
 $(AAREOBJECTS):
-	make -C $(AAREDIR) CFLAGS="$(EXTRA_CFLAGS)"
+	make -C $(AAREDIR) CFLAGS="$(EXTRA_CXXFLAGS)"
 
 .PHONY: install-rhel4
 install-rhel4: install-redhat

=== modified file 'parser/libapparmor_re/flex-tables.h'
--- parser/libapparmor_re/flex-tables.h	2008-03-13 16:46:19 +0000
+++ parser/libapparmor_re/flex-tables.h	2010-11-09 21:10:56 +0000
@@ -11,8 +11,8 @@
 	uint32_t	th_hsize;
 	uint32_t	th_ssize;
 	uint16_t	th_flags;
-	char		th_version[];
-/*	char		th_name[];
+/*	char		th_version[];
+	char		th_name[];
 	char		th_pad64[];*/
 } __attribute__ ((packed));
 
@@ -34,8 +34,8 @@
 	uint16_t	td_flags;
 	uint32_t	td_hilen;
 	uint32_t	td_lolen;
-	char		td_data[];
-/*	char		td_pad64[];*/
+/*	char		td_data[];
+	char		td_pad64[];*/
 } __attribute__ ((packed));
 
 #endif

=== modified file 'parser/libapparmor_re/regexp.y'
--- parser/libapparmor_re/regexp.y	2010-11-09 19:57:43 +0000
+++ parser/libapparmor_re/regexp.y	2010-11-09 21:12:05 +0000
@@ -351,7 +351,7 @@
 	   */
 	}
 
-	void follow(NodeCases& cases)
+	void follow(NodeCases& cases __attribute__((unused)))
 	{
 	    /* Nothing to follow. */
 	}
@@ -936,7 +936,7 @@
 	bool update;
 
 	if (flags & DFA_DUMP_TREE_STATS) {
-		struct node_counts counts = { };
+		struct node_counts counts = { 0, 0, 0, 0, 0, 0, 0, 0 };
 		count_tree_nodes(t, &counts);
 		fprintf(stderr, "expr tree: c %d, [] %d, [^] %d, | %d, + %d, * %d, . %d, cat %d\n", counts.charnode, counts.charset, counts.notcharset, counts.alt, counts.plus, counts.star, counts.any, counts.cat);
 	}
@@ -968,7 +968,7 @@
 		}
 	} while(update);
 	if (flags & DFA_DUMP_TREE_STATS) {
-		struct node_counts counts = { };
+		struct node_counts counts = { 0, 0, 0, 0, 0, 0, 0, 0 };
 		count_tree_nodes(t, &counts);
 		fprintf(stderr, "simplified expr tree: c %d, [] %d, [^] %d, | %d, + %d, * %d, . %d, cat %d\n", counts.charnode, counts.charset, counts.notcharset, counts.alt, counts.plus, counts.star, counts.any, counts.cat);
 	}
@@ -1286,7 +1286,9 @@
 }
 
 void
-regexp_error(Node **, const char *text, const char *error)
+regexp_error(Node ** __attribute__((unused)),
+	     const char *text __attribute__((unused)),
+	     const char *error __attribute__((unused)))
 {
     /* We don't want the library to print error messages. */
 }
@@ -2304,7 +2306,7 @@
 /**
  * Does <cases> fit into position <base> of the transition table?
  */
-bool TransitionTable::fits_in(vector <pair<size_t, size_t> > &free_list,
+bool TransitionTable::fits_in(vector <pair<size_t, size_t> > &free_list __attribute__((unused)),
 			      size_t pos, Cases& cases)
 {
 	size_t c, base = pos - cases.begin()->first;
@@ -2508,7 +2510,7 @@
 template<class Iter>
 void write_flex_table(ostream& os, int id, Iter pos, Iter end)
 {
-    struct table_header td = { };
+    struct table_header td = { 0, 0, 0, 0 };
     size_t size = end - pos;
 
     td.td_id = htons(id);
@@ -2534,7 +2536,7 @@
 void TransitionTable::flex_table(ostream& os, const char *name)
 {
     const char th_version[] = "notflex";
-    struct table_set_header th = { };
+    struct table_set_header th = { 0, 0, 0, 0 };
 
     /**
      * Change the following two data types to adjust the maximum flex

=== modified file 'parser/parser_lex.l'
--- parser/parser_lex.l	2010-09-14 19:22:02 +0000
+++ parser/parser_lex.l	2010-11-09 20:34:51 +0000
@@ -22,6 +22,7 @@
 
 /* eliminates need to link with libfl */
 %option noyywrap
+%option nounput
 
 %{
 #include <stdio.h>

=== modified file 'parser/parser_regex.c'
--- parser/parser_regex.c	2010-07-31 23:00:52 +0000
+++ parser/parser_regex.c	2010-11-09 20:37:54 +0000
@@ -35,38 +35,6 @@
 	e_buffer_overflow
 };
 
-/* filter_escapes: scan input for any escape characters
- * and remove, and reduce double \\ to a single
- * NOTE: modifies in place the contents of the name argument */
-static void filter_escapes(char *name)
-{
-	char *sptr, *dptr;
-	BOOL bEscape = 0;
-
-	if (!name)	/* shouldn't happen */
-		return;
-
-	sptr = dptr = name;
-	while (*sptr) {
-		if (*sptr == '\\') {
-			if (bEscape) {
-				*dptr++ = *sptr++;
-			} else {
-				++sptr;
-				bEscape = TRUE;
-				continue;
-			}
-		} else if (dptr < sptr) {
-			*dptr++ = *sptr++;
-		} else {
-			dptr++;
-			sptr++;
-		}
-		bEscape = 0;
-	}
-	*dptr = 0;
-}
-
 /* Filters out multiple slashes (except if the first two are slashes,
  * that's a distinct namespace in linux) and trailing slashes.
  * NOTE: modifies in place the contents of the path argument */
@@ -661,21 +629,6 @@
 /* Guh, fake symbol */
 char *progname;
 
-static int test_filter_escapes(void)
-{
-	int rc = 0;
-	char *test_string;
-
-	test_string = strdup("foo\\\\foo");
-	filter_escapes(test_string);
-	MY_TEST(strcmp(test_string, "foo\\foo") == 0, "simple filter for \\\\");
-
-	test_string = strdup("foo\\foo");
-	filter_escapes(test_string);
-	MY_TEST(strcmp(test_string, "foofoo") == 0, "simple filter for \\f");
-	return rc;
-}
-
 static int test_filter_slashes(void)
 {
 	int rc = 0;
@@ -725,10 +678,6 @@
 	int rc = 0;
 	int retval;
 
-	retval = test_filter_escapes();
-	if (retval != 0)
-		rc = retval;
-
 	retval = test_filter_slashes();
 	if (retval != 0)
 		rc = retval;


-- 
Kees Cook
Ubuntu Security Team



More information about the AppArmor mailing list