[apparmor] [PATCH] clean up warnings from parser rework
John Johansen
john.johansen at canonical.com
Tue Nov 9 21:32:22 GMT 2010
On 11/09/2010 01:18 PM, Kees Cook wrote:
> 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;
>
>
ACK, though I could have sworn that I pulled filter escapes out before
More information about the AppArmor
mailing list