[apparmor] [patch} parser: convert array into unordered map (was Re: [Branch ~apparmor-dev/apparmor/master] Rev 2193: Convert the parser to C++)

[Steve Beattie]

On Fri, Nov 08, 2013 at 02:59:23PM -0600, Jamie Strandboge wrote:
> I just now noticed that the daily builds using gcc/g++ 4.6 started failing,
> presumably after this patches series:
> https://launchpadlibrarian.net/151700403/buildlog_ubuntu-precise-i386.apparmor_2.7.104%2B2200%2B12~ubuntu12.04.1_FAILEDTOBUILD.txt.gz

Yes, the conversion to C++ did break with g++ 4.6 due to the designated
array initializer used for the state_names array in parser_lex.l. 
> gcc/g++ 4.7 were fine with this patchset:
> https://launchpadlibrarian.net/155989184/buildlog_ubuntu-quantal-i386.apparmor_2.7.104%2B2249%2B12~ubuntu12.10.1_UPLOADING.txt.gz
> 
> The question becomes, do we care about gcc/g++ 4.6? If so, we should fix this
> (it seems we might be able to fiddle with build flags to make it work, but I'll
> let others dive in). If not, we should update our builds to fail with 4.6.

Unfortunately, it's not as simple as fiddling with make flags, it still
fails to compile with g++ 4.6 even when the -std=gnu++0x flag is given.

That said, I think continuing to support g++ 4.6 is important, given
it's the compiler version in Ubuntu 12.04 LTS (aka precise). So here
is a patch that converts the problematic array definition into a C++
unordered_map type. Using this depends on using the c++0x (aka c++11)
standard, and as we have gnuisms elsewhere (using the typeof builtin),
the patch also adds/converts to using -std=gnu++c0x in the build
rules (which conveniently eliminates some other warnings we had due
to other c++11-isms).

Signed-off-by: Steve Beattie <steve at nxnw.org>
---
 parser/Makefile                |    2 +-
 parser/libapparmor_re/Makefile |    4 ++--
 parser/parser_lex.l            |   19 +++++++++++--------
 3 files changed, 14 insertions(+), 11 deletions(-)

Index: b/parser/Makefile
===================================================================
--- a/parser/Makefile
+++ b/parser/Makefile
@@ -55,7 +55,7 @@ endif #CFLAGS
 
 LIBAPPARMOR_PATH=../libraries/libapparmor/src/
 LIBAPPARMOR_LDPATH=$(LIBAPPARMOR_PATH)/.libs/
-EXTRA_CXXFLAGS = ${CFLAGS} ${CXX_WARNINGS} -D_GNU_SOURCE -I$(LIBAPPARMOR_PATH)
+EXTRA_CXXFLAGS = ${CFLAGS} ${CXX_WARNINGS} -std=gnu++0x -D_GNU_SOURCE -I$(LIBAPPARMOR_PATH)
 EXTRA_CFLAGS = ${EXTRA_CXXFLAGS} ${CPP_WARNINGS}
 
 #LEXLIB	:= -lfl
Index: b/parser/parser_lex.l
===================================================================
--- a/parser/parser_lex.l
+++ b/parser/parser_lex.l
@@ -34,6 +34,10 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <dirent.h>
+
+#include <unordered_map>
+#include <string>
+
 #define _(s) gettext(s)
 
 #include "parser.h"
@@ -48,7 +52,7 @@
 /* #define DEBUG */
 #ifdef DEBUG
 static int yy_top_state(void);
-#define PDEBUG(fmt, args...) printf("Lexer (Line %d) (state %s): " fmt, current_lineno, state_names[YY_START], ## args)
+#define PDEBUG(fmt, args...) printf("Lexer (Line %d) (state %s): " fmt, current_lineno, state_names[YY_START].c_str(), ## args)
 #else
 #define PDEBUG(fmt, args...)	/* Do nothing */
 #endif
@@ -72,13 +76,13 @@ do { \
 
 #define POP() \
 do { \
-	DUMP_AND_DEBUG(" (pop_to(%s)): Matched: %s\n", state_names[yy_top_state()], yytext); \
+	DUMP_AND_DEBUG(" (pop_to(%s)): Matched: %s\n", state_names[yy_top_state()].c_str(), yytext); \
 	yy_pop_state(); \
 } while (0)
 
 #define PUSH(X) \
 do { \
-	DUMP_AND_DEBUG(" (push(%s)): Matched: %s\n", state_names[(X)], yytext); \
+	DUMP_AND_DEBUG(" (push(%s)): Matched: %s\n", state_names[(X)].c_str(), yytext); \
 	yy_push_state(X); \
 } while (0)
 
@@ -96,7 +100,7 @@ do { \
 
 #define BEGIN_AND_RETURN(X, Y) \
 do { \
-	DUMP_AND_DEBUG(" (begin(%s)): Matched: %s\n", state_names[(X)], yytext); \
+	DUMP_AND_DEBUG(" (begin(%s)): Matched: %s\n", state_names[(X)].c_str(), yytext); \
 	BEGIN(X); \
 	return (Y); \
 } while (0)
@@ -104,9 +108,8 @@ do { \
 
 #define YY_NO_INPUT
 
-#define STATE_TABLE_ENT(X) [(X)] = #X
-/* static char *const state_names[]; */
-
+#define STATE_TABLE_ENT(X) {X, #X }
+extern unordered_map<int, string> state_names;
 
 struct cb_struct {
 	const char *fullpath;
@@ -591,7 +594,7 @@ LT_EQUAL	<=
 /* Create a table mapping lexer state number to the name used in the
  * in the code.  This allows for better debug output
  */
-static const char *const state_names[] = {
+unordered_map<int, string> state_names = {
 	STATE_TABLE_ENT(INITIAL),
 	STATE_TABLE_ENT(SUB_ID),
 	STATE_TABLE_ENT(SUB_VALUE),
Index: b/parser/libapparmor_re/Makefile
===================================================================
--- a/parser/libapparmor_re/Makefile
+++ b/parser/libapparmor_re/Makefile
@@ -3,8 +3,8 @@
 
 TARGET=libapparmor_re.a
 
-CFLAGS ?= -g -Wall -O2 ${EXTRA_CFLAGS}
-CXXFLAGS := ${CFLAGS} -std=c++0x
+CFLAGS ?= -g -Wall -O2 ${EXTRA_CFLAGS} -std=gnu++0x
+CXXFLAGS := ${CFLAGS}
 
 ARFLAGS=-rcs
 

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20131116/30c4502e/attachment.pgp>