From e4e6d8845d801c37991a71424df7fad8d91cc783 Mon Sep 17 00:00:00 2001
From: Andy Dalton <751263+adalton@users.noreply.github.com>
Date: Tue, 27 Nov 2018 20:38:10 -0500
Subject: [PATCH] Add optional support for -Wextra and -Werror (#479)

The -Wextra compile-time option will enable additional diagnostic
warnigns.  The -Werror option will cause the compiler to treat warnings
as errors.  This change adds a build time option,
BUILD_WARNINGS_AS_ERRORS, to conditionally enable those flags.  Note
that depending on the compiler you're using, if you enable this option,
compilation may fail (some compiler version have additional warnings
that have not yet been resolved).

Testing with these options in place identified a destructor that was
throwing an exception.  C++11 doesn't allow destructors to throw
exceptions, so those throw's would have resulted in calls to
terminate().  I replace them with an error log and a call to assert().
---
 CMakeLists.txt                    | 13 +++++++++++--
 userspace/falco/falco_outputs.cpp | 11 +++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3c7e965d9d9..662dab91b5a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -19,6 +19,8 @@ cmake_minimum_required(VERSION 2.8.2)
 
 project(falco)
 
+option(BUILD_WARNINGS_AS_ERRORS "Enable building with -Wextra -Werror flags")
+
 if(NOT DEFINED FALCO_VERSION)
 	set(FALCO_VERSION "0.1.1dev")
 endif()
@@ -35,8 +37,15 @@ if(NOT DRAIOS_DEBUG_FLAGS)
 	set(DRAIOS_DEBUG_FLAGS "-D_DEBUG")
 endif()
 
-set(CMAKE_C_FLAGS "-Wall -ggdb ${DRAIOS_FEATURE_FLAGS}")
-set(CMAKE_CXX_FLAGS "-Wall -ggdb --std=c++0x ${DRAIOS_FEATURE_FLAGS}")
+set(CMAKE_COMMON_FLAGS "-Wall -ggdb ${DRAIOS_FEATURE_FLAGS}")
+
+if(BUILD_WARNINGS_AS_ERRORS)
+	set(CMAKE_SUPPRESSED_WARNINGS "-Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare -Wno-type-limits -Wno-implicit-fallthrough -Wno-format-truncation")
+	set(CMAKE_COMMON_FLAGS "${CMAKE_COMMON_FLAGS} -Wextra -Werror ${CMAKE_SUPPRESSED_WARNINGS}")
+endif()
+
+set(CMAKE_C_FLAGS "${CMAKE_COMMON_FLAGS}")
+set(CMAKE_CXX_FLAGS "--std=c++0x ${CMAKE_COMMON_FLAGS}")
 
 set(CMAKE_C_FLAGS_DEBUG "${DRAIOS_DEBUG_FLAGS}")
 set(CMAKE_CXX_FLAGS_DEBUG "${DRAIOS_DEBUG_FLAGS}")
diff --git a/userspace/falco/falco_outputs.cpp b/userspace/falco/falco_outputs.cpp
index 3ef21d620ef..97341e0b1b8 100644
--- a/userspace/falco/falco_outputs.cpp
+++ b/userspace/falco/falco_outputs.cpp
@@ -37,19 +37,26 @@ falco_outputs::falco_outputs(falco_engine *engine)
 
 falco_outputs::~falco_outputs()
 {
+	// Note: The assert()s in this destructor were previously places where
+	//       exceptions were thrown.  C++11 doesn't allow destructors to
+	//       emit exceptions; if they're thrown, they'll trigger a call
+	//       to 'terminate()'.  To maintain similar behavior, the exceptions
+	//       were replace with calls to 'assert()'
 	if(m_initialized)
 	{
 		lua_getglobal(m_ls, m_lua_output_cleanup.c_str());
 
 		if(!lua_isfunction(m_ls, -1))
 		{
-			throw falco_exception("No function " + m_lua_output_cleanup + " found. ");
+			falco_logger::log(LOG_ERR, std::string("No function ") + m_lua_output_cleanup + " found. ");
+			assert(nullptr == "Missing lua cleanup function in ~falco_outputs");
 		}
 
 		if(lua_pcall(m_ls, 0, 0, 0) != 0)
 		{
 			const char* lerr = lua_tostring(m_ls, -1);
-			throw falco_exception(string(lerr));
+			falco_logger::log(LOG_ERR, std::string("lua_pcall failed, err: ") + lerr);
+			assert(nullptr == "lua_pcall failed in ~falco_outputs");
 		}
 	}
 }