Clang c++ compilation fixes

.github/workflows/devshell.yml

@@ -39,6 +39,7 @@ jobs:
           # Setup build time environment variables
           PYTHONUSERBASE="${HOME}/python_packages"
           CC="${{ matrix.cc }}"
+          CXX=`[ $CC = gcc ] && echo g++ || echo clang++`
           SYSLOG_NG_INSTALL_DIR=${HOME}/install/syslog-ng
           CONFIGURE_FLAGS="
             --prefix=${SYSLOG_NG_INSTALL_DIR}
@@ -56,7 +57,7 @@ jobs:
             -DCMAKE_INSTALL_PREFIX=${HOME}/install/syslog-ng
             -DPYTHON_VERSION=3
           "
-          gh_export COREFILES_DIR PYTHONUSERBASE CC SYSLOG_NG_INSTALL_DIR CONFIGURE_FLAGS CMAKE_FLAGS
+          gh_export COREFILES_DIR PYTHONUSERBASE CC CXX SYSLOG_NG_INSTALL_DIR CONFIGURE_FLAGS CMAKE_FLAGS
           gh_path "${PYTHONUSERBASE}"
 
       - name: autogen.sh

CMakeLists.txt

@@ -229,17 +229,32 @@ find_package(BISON 3.7.6 REQUIRED)
 
 string(REGEX MATCH "^([0-9]+)\\.([0-9]+)\\..*$" _dummy "${BISON_VERSION}")
 
-set(GLIB_COMPILE_DEFINITIONS
-  GLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_32
-)
 find_package(GLIB 2.32.0 REQUIRED COMPONENTS gmodule gthread)
 
 set(CMAKE_REQUIRED_INCLUDES ${GLIB_INCLUDE_DIRS})
 set(CMAKE_REQUIRED_LIBRARIES ${GLIB_LIBRARIES})
 check_symbol_exists(g_list_copy_deep "glib.h" SYSLOG_NG_HAVE_G_LIST_COPY_DEEP)
+check_symbol_exists(g_memdup2 "glib.h" SYSLOG_NG_HAVE_MEMDUP2)
 check_symbol_exists(g_ptr_array_find_with_equal_func "glib.h" SYSLOG_NG_HAVE_G_PTR_ARRAY_FIND_WITH_EQUAL_FUNC)
 check_symbol_exists(g_canonicalize_filename "glib.h" SYSLOG_NG_HAVE_G_CANONICALIZE_FILENAME)
 
+# only set MIN_REQUIRED to 2.68 if we know that we are at least on that version.
+# On CentOS7/SLES12 we don't have recent enough glib but with compat
+# wrappers we can compile. Bad news is that if we don't set to 2.68 at least,
+# we can't compile on new platforms with clang.
+#
+# Best of both worlds: we set to 2.68 if we have that version, so we
+# get clang to compile our code, otherwise we set it to 2.32.
+if (PC_GLIB_VERSION VERSION_GREATER_EQUAL "2.68")
+
+  set_target_properties(GLib::GLib PROPERTIES
+        INTERFACE_COMPILE_DEFINITIONS "GLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_68")
+
+else()
+  set_target_properties(GLib::GLib PROPERTIES
+        INTERFACE_COMPILE_DEFINITIONS "GLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_32")
+endif()
+
 find_package(RabbitMQ)
 if (RabbitMQ_FOUND)
   set(CMAKE_REQUIRED_INCLUDES ${RabbitMQ_INCLUDE_DIR})

Makefile.am

@@ -77,8 +77,9 @@ AM_CXXFLAGS += \
 if ENABLE_EXTRA_WARNINGS
 AM_CFLAGS += \
 	-Wimplicit-function-declaration \
+	$(CFLAGS_W_NO_INITIALIZER_OVERRIDES) \
+	$(CFLAGS_W_OLD_STYLE_DECLARATION) \
 	-Wnested-externs \
-	-Wold-style-declaration \
 	-Wstrict-prototypes \
 	-Wswitch-default \
 	-Wimplicit-int \

configure.ac

@@ -481,14 +481,6 @@ dnl C++
 # Ok, here is the deal
 # clang cannot compile the current source, it has nultiple issues
 # replace it till not fixed them
-if test "x$CXX" = "xclang"; then
-        AC_MSG_ERROR(["clang cannot compile syslog-ng C++ sources, try using gcc instead (export CXX=gcc) for C++ compilation (NOTE: on macOS you need Homebrew gcc not the Apple one)"])
-elif test "x$CC" = "xclang"; then
-        CPP_COMPILER="gcc"
-        AC_MSG_WARN(["clang cannot compile syslog-ng C++ sources, switched to $CPP_COMPILER for C++ compilation (NOTE: on macOS you need Homebrew gcc not the Apple one)"])
-else
-        CPP_COMPILER="$CXX $CC"
-fi
 # This one is useful for checking the available compilers
 # AC_MSG_RESULT(gcc = $(which gcc) - g++ = $(which g++) - clang = $(which clang))
 # AC_MSG_RESULT(Paths: gcc = $(ls -Al $(which gcc)) - g++ = $(ls -Al $(which g++)) - clang = $(ls -Al $(which clang)))
@@ -630,9 +622,10 @@ dnl We are checking for the postive warning flag, as gcc handles the
 dnl -Wno-XXX version oddly, so if no other warnings are present, it is not
 dnl reported at all, causing it to be unusable for detection purposes.
 
-AX_CFLAGS_GCC_OPTION(-Winitializer-overrides,
-		     CFLAGS_INITIALIZER_OVERRIDES,
-		     CFLAGS_ADD="${CFLAGS_ADD} -Wno-initializer-overrides")
+AX_CFLAGS_GCC_OPTION(-Winitializer-overrides, CFLAGS_W_INITIALIZER_OVERRIDES, CFLAGS_W_NO_INITIALIZER_OVERRIDES="-Wno-initializer-overrides")
+AX_CFLAGS_GCC_OPTION(-Wold-style-declaration, CFLAGS_W_OLD_STYLE_DECLARATION)
+AC_SUBST(CFLAGS_W_NO_INITIALIZER_OVERRIDES)
+AC_SUBST(CFLAGS_W_OLD_STYLE_DECLARATION)
 
 dnl User supplied CFLAGS come last
 CFLAGS="${CFLAGS_ADD} ${ac_cv_env_CFLAGS_value}"
@@ -1033,8 +1026,6 @@ if test "$linking_mode" != "dynamic"; then
 	LIBS=$old_LIBS
 fi
 
-GLIB_CFLAGS="${GLIB_CFLAGS} -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_32"
-
 old_CPPFLAGS=$CPPFLAGS
 CPPFLAGS="$GLIB_CFLAGS"
 old_LDFLAGS=$LDFLAGS
@@ -1044,9 +1035,24 @@ old_LIBS=$LIBS
 LIBS="$LIBS $GLIB_LIBS"
 AC_CHECK_FUNCS(g_list_copy_deep \
   g_ptr_array_find_with_equal_func \
+  g_memdup2 \
   g_canonicalize_filename)
 LIBS=$old_LIBS
 
+if test "x$ac_cv_func_g_memdup2" = "xyes"; then
+    dnl only set MIN_REQUIRED to 2.68 if we know that we are at least on that version.
+    dnl On CentOS7/SLES12 we don't have recent enough glib but with compat
+    dnl wrappers we can compile. Bad news is that if we don't set to 2.68 at least,
+    dnl we can't compile on new platforms with clang.
+
+    dnl Best of both worlds: we set to 2.68 if we have that version, so we
+    dnl get clang to compile our code, otherwise we set it to 2.32.
+
+    GLIB_CFLAGS="${GLIB_CFLAGS} -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_68"
+else
+    GLIB_CFLAGS="${GLIB_CFLAGS} -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_32"
+fi
+
 AC_CACHE_CHECK(sanity checking Glib headers,
                blb_cv_glib_sane,
 [AC_TRY_RUN([

lib/cfg-grammar.y

@@ -59,6 +59,8 @@
 %code {
 
 #pragma GCC diagnostic ignored "-Wswitch-default"
+#pragma GCC diagnostic ignored "-Wunused-but-set-variable"
+
 #if (defined(__GNUC__) && __GNUC__ >= 6) || (defined(__clang__) && __clang_major__ >= 10)
 #  pragma GCC diagnostic ignored "-Wmisleading-indentation"
 #endif

lib/compat/cpp-start.h

@@ -62,6 +62,28 @@
  *     functions with wrapper functions of your real C++ functions in the
  *     ctor of the C style "class" == struct.
  *
+ * Header ordering:
+ *   - syslog-ng.h needs to come first even in cpp/hpp files.  You don't
+ *     need cpp-start/end wrapper around syslog-ng.h.  This include can
+ *     happen indirectly, e.g.  if you include any other header that should
+ *     take care of syslog-ng.h
+ *
+ *   - syslog-ng.h will include glib too, which is also C++ safe.  This is
+ *     needed as some constructs in glib only work if it is outside of an
+ *     extern "C" block.
+ *
+ *   - Use the usual header ordering conventions, e.g.  include the closest
+ *     header first (e.g.  the one associated with your module) and then
+ *     iterate to furthest.  This ensures that each header is actually
+ *     standalone and includes all its dependencies.
+ *
+ *   - If you use a .hpp file, include .h from it and make sure the .h
+ *     includes syslog-ng.h and includes all other .h files by wrapping them
+ *     using cpp-start/end.h as needed. This takes care of the first rule.
+ *
+ *   - In your .cpp files, include the .hpp file but not the .h file which is
+ *     already included.
+ *
  * You can see an example usage of the C++ plugin support at:
  *      modules/examples/sources/random-choice-generator
  */

lib/compat/glib.h

@@ -59,4 +59,13 @@ gboolean slng_g_hash_table_insert (GHashTable *hash_table, gpointer key, gpointe
 gunichar g_utf8_get_char_validated_fixed (const gchar *p, gssize max_len);
 #endif
 
+#if !GLIB_CHECK_VERSION(2, 68, 0)
+#define g_memdup2 g_memdup
+#endif
+
+#if !GLIB_CHECK_VERSION(2, 70, 0)
+#define g_pattern_spec_match_string g_pattern_match_string
+#define g_pattern_spec_match g_pattern_match
+#endif
+
 #endif

lib/logmatcher.c

@@ -244,7 +244,7 @@ log_matcher_glob_match(LogMatcher *s, LogMessage *msg, gint value_handle, const
           warned = TRUE;
         }
       APPEND_ZERO(buf, value, value_len);
-      return g_pattern_match(self->pattern, value_len, buf, NULL);
+      return g_pattern_spec_match(self->pattern, value_len, buf, NULL);
     }
   else
     {

lib/logmsg/logmsg.c

@@ -970,7 +970,7 @@ log_msg_set_tag_by_id_onoff(LogMessage *self, LogTagId id, gboolean on)
             evt_tag_printf("msg", "%p", self));
   if (!log_msg_chk_flag(self, LF_STATE_OWN_TAGS) && self->num_tags)
     {
-      self->tags = g_memdup(self->tags, sizeof(self->tags[0]) * self->num_tags);
+      self->tags = g_memdup2(self->tags, sizeof(self->tags[0]) * self->num_tags);
     }
   log_msg_set_flag(self, LF_STATE_OWN_TAGS);
 

lib/scanner/kv-scanner/tests/test_kv_scanner.c

@@ -825,7 +825,7 @@ Try to increase stolen memory size if available in BIOS.",
     {}
   };
 
-  return g_memdup(tc, sizeof(tc));
+  return g_memdup2(tc, sizeof(tc));
 }
 
 static Testcase *
@@ -964,7 +964,7 @@ _provide_cases_for_performance_test_parse_long_msg(void)
     },
     {}
   };
-  return g_memdup(tc, sizeof(tc));
+  return g_memdup2(tc, sizeof(tc));
 }
 
 #define ITERATION_NUMBER 100000

lib/scanner/xml-scanner/xml-scanner.c

@@ -34,7 +34,7 @@
   the same matchstring must be matched against different patterns,
   because memory is allocated each time and string is copied as
   reversed, though it would be enough to execute this reverse once. For
-  that reason one can use g_pattern_match(), which accepts both the
+  that reason one can use g_pattern_spec_match(), which accepts both the
   matchstring and a reversed matchstring as parameters.
 
   Though there are cases when no reverse is needed at all. This is
@@ -150,8 +150,8 @@ tag_matches_patterns(const GPtrArray *patterns, const gint tag_length,
                      const gchar *element_name, const gchar *reversed_name)
 {
   for (int i = 0; i < patterns->len; i++)
-    if (g_pattern_match((GPatternSpec *)g_ptr_array_index(patterns, i),
-                        tag_length, element_name, reversed_name))
+    if (g_pattern_spec_match((GPatternSpec *)g_ptr_array_index(patterns, i),
+                             tag_length, element_name, reversed_name))
       {
         return TRUE;
       }

lib/stats/stats-query.c

@@ -45,7 +45,7 @@ static gboolean
 _is_pattern_matches_key(GPatternSpec *pattern, gpointer key)
 {
   gchar *counter_name = (gchar *) key;
-  return g_pattern_match_string(pattern, counter_name);
+  return g_pattern_spec_match_string(pattern, counter_name);
 }
 
 static gboolean

lib/value-pairs/transforms.c

@@ -305,7 +305,7 @@ value_pairs_transform_set_free(ValuePairsTransformSet *vpts)
 void
 value_pairs_transform_set_apply(ValuePairsTransformSet *vpts, GString *key)
 {
-  if (g_pattern_match_string(vpts->pattern, key->str))
+  if (g_pattern_spec_match_string(vpts->pattern, key->str))
     {
       GList *l;
 

lib/value-pairs/value-pairs.c

@@ -142,7 +142,7 @@ static CfgFlagHandler value_pair_scope[] =
 static gboolean
 vp_pattern_spec_eval(VPPatternSpec *self, const gchar *input)
 {
-  return g_pattern_match_string(self->pattern, input);
+  return g_pattern_spec_match_string(self->pattern, input);
 }
 
 static void

modules/add-contextual-data/add-contextual-data-glob-selector.c

@@ -99,7 +99,7 @@ _find_first_matching_glob(AddContextualDataGlobSelector *self, LogMessage *msg)
   for (gint i = 0; i < self->globs->len; i++)
     {
       GlobExpression *gs = &g_array_index(self->globs, GlobExpression, i);
-      gboolean result = g_pattern_match(gs->expr, string->len, string->str, string_reversed->str);
+      gboolean result = g_pattern_spec_match(gs->expr, string->len, string->str, string_reversed->str);
 
       msg_trace("add-contextual-data(): Evaluating glob against message",
                 evt_tag_str("glob-template", self->glob_template->template_str),

modules/affile/wildcard-source.c

@@ -136,7 +136,7 @@ _create_file_reader(WildcardSourceDriver *self, const gchar *full_path)
 static void
 _handle_file_created(WildcardSourceDriver *self, const DirectoryMonitorEvent *event)
 {
-  if (g_pattern_match_string(self->compiled_pattern, event->name))
+  if (g_pattern_spec_match_string(self->compiled_pattern, event->name))
     {
       WildcardFileReader *reader = g_hash_table_lookup(self->file_readers, event->full_path);
 

modules/basicfuncs/list-funcs.c

@@ -445,7 +445,7 @@ string_matcher_match(StringMatcher *self, const char *string, gsize string_len)
     case SMM_SUBSTRING:
       return (strstr(string, self->pattern) != NULL);
     case SMM_GLOB:
-      return (g_pattern_match_string(self->glob, string));
+      return (g_pattern_spec_match_string(self->glob, string));
     case SMM_PCRE:
       return (string_matcher_match_pcre(self, string, string_len));
     default:

modules/cloud-auth/cloud-auth.h

@@ -23,6 +23,10 @@
 #ifndef CLOUD_AUTH_H
 #define CLOUD_AUTH_H
 
+#include "syslog-ng.h"
+
+#include "compat/cpp-start.h"
+
 #include "logqueue.h"
 #include "driver.h"
 
@@ -44,4 +48,7 @@ typedef struct _CloudAuthDestPlugin CloudAuthDestPlugin;
 LogDriverPlugin *cloud_auth_dest_plugin_new(void);
 void cloud_auth_dest_plugin_set_authenticator(LogDriverPlugin *s, CloudAuthenticator *authenticator);
 
+#include "compat/cpp-end.h"
+
+
 #endif

modules/cloud-auth/cloud-auth.hpp

@@ -23,9 +23,7 @@
 #ifndef CLOUD_AUTH_HPP
 #define CLOUD_AUTH_HPP
 
-#include "compat/cpp-start.h"
 #include "cloud-auth.h"
-#include "compat/cpp-end.h"
 
 namespace syslogng {
 namespace cloud_auth {

modules/cloud-auth/google-auth.cpp

@@ -22,14 +22,14 @@
 
 #include "google-auth.hpp"
 
-#include <exception>
-#include <fstream>
-#include <cmath>
-
 #include "compat/cpp-start.h"
 #include "scratch-buffers.h"
 #include "compat/cpp-end.h"
 
+#include <exception>
+#include <fstream>
+#include <cmath>
+
 using namespace syslogng::cloud_auth::google;
 
 ServiceAccountAuthenticator::ServiceAccountAuthenticator(const char *key_path, const char *audience_,

modules/cloud-auth/google-auth.h

@@ -25,6 +25,8 @@
 
 #include "cloud-auth.h"
 
+#include "compat/cpp-start.h"
+
 typedef enum _GoogleAuthenticatorAuthMode
 {
   GAAM_UNDEFINED,
@@ -44,5 +46,6 @@ void google_authenticator_set_user_managed_service_account_name(CloudAuthenticat
 void google_authenticator_set_user_managed_service_account_metadata_url(CloudAuthenticator *s,
     const gchar *metadata_url);
 
+#include "compat/cpp-end.h"
 
 #endif

modules/cloud-auth/google-auth.hpp

@@ -23,10 +23,7 @@
 #ifndef GOOGLE_AUTH_HPP
 #define GOOGLE_AUTH_HPP
 
-#include "compat/cpp-start.h"
 #include "google-auth.h"
-#include "compat/cpp-end.h"
-
 #include "cloud-auth.hpp"
 
 #include <mutex>

modules/correlation/tests/test_timer_wheel.c

@@ -93,11 +93,11 @@ test_wheel(gint seed)
         latest = expires;
 
       timer1 = timer_wheel_add_timer(wheel, expires - 1, timer_callback,
-                                     g_memdup(&expires, sizeof(expires)), (GDestroyNotify) g_free);
+                                     g_memdup2(&expires, sizeof(expires)), (GDestroyNotify) g_free);
       timer2 = timer_wheel_add_timer(wheel, expires - 1, timer_callback,
-                                     g_memdup(&expires, sizeof(expires)), (GDestroyNotify) g_free);
+                                     g_memdup2(&expires, sizeof(expires)), (GDestroyNotify) g_free);
       timer3 = timer_wheel_add_timer(wheel, expires - 1, timer_callback,
-                                     g_memdup(&expires, sizeof(expires)), (GDestroyNotify) g_free);
+                                     g_memdup2(&expires, sizeof(expires)), (GDestroyNotify) g_free);
       expected_callbacks += 3;
       r = rand() & 0xFF;
       if (r < 64)

modules/examples/sources/random-choice-generator/random-choice-generator.cpp

@@ -20,15 +20,14 @@
  *
  */
 
-#include <unistd.h>
-
-#include "random-choice-generator.h"
 #include "random-choice-generator.hpp"
 
 #include "compat/cpp-start.h"
 #include "string-list.h"
 #include "compat/cpp-end.h"
 
+#include <unistd.h>
+
 #define get_SourceDriver(s) (((RandomChoiceGeneratorSourceDriver *) (s))->cpp)
 #define get_SourceWorker(s) (((RandomChoiceGeneratorSourceWorker *) (s))->cpp)