Fix: define macros for logging levels
authorSimon Marchi <simon.marchi@efficios.com>
Mon, 21 Oct 2019 17:59:50 +0000 (13:59 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Wed, 30 Oct 2019 19:14:53 +0000 (15:14 -0400)
Compiling with -Wundef, we get:

      CC       common.lo
    In file included from /home/smarchi/src/babeltrace/src/common/common.c:27:0:
    /home/smarchi/src/babeltrace/src/logging/log.h:58:24: warning: "BT_LOGGING_LEVEL_TRACE" is not defined, evaluates to 0 [-Wundef]
     #define BT_LOG_TRACE   BT_LOGGING_LEVEL_TRACE
                            ^
    /home/smarchi/src/babeltrace/src/logging/log.h:617:35: note: in definition of macro ‘BT_LOG_ENABLED’
     #define BT_LOG_ENABLED(lvl)     ((lvl) >= _BT_MINIMAL_LOG_LEVEL)
                                       ^~~
    /home/smarchi/src/babeltrace/src/logging/log.h:618:48: note: in expansion of macro ‘BT_LOG_TRACE’
     #define BT_LOG_ENABLED_TRACE    BT_LOG_ENABLED(BT_LOG_TRACE)
                                                    ^~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:852:5: note: in expansion of macro ‘BT_LOG_ENABLED_TRACE’
     #if BT_LOG_ENABLED_TRACE
         ^~~~~~~~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:59:24: warning: "BT_LOGGING_LEVEL_DEBUG" is not defined, evaluates to 0 [-Wundef]
     #define BT_LOG_DEBUG   BT_LOGGING_LEVEL_DEBUG
                            ^
    ../../src/common/config.h:26:30: note: in expansion of macro ‘BT_LOG_DEBUG’
     #define BT_MINIMAL_LOG_LEVEL BT_LOG_DEBUG
                                  ^~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:92:32: note: in expansion of macro ‘BT_MINIMAL_LOG_LEVEL’
      #define _BT_MINIMAL_LOG_LEVEL BT_MINIMAL_LOG_LEVEL
                                    ^~~~~~~~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:617:43: note: in expansion of macro ‘_BT_MINIMAL_LOG_LEVEL’
     #define BT_LOG_ENABLED(lvl)     ((lvl) >= _BT_MINIMAL_LOG_LEVEL)
                                               ^~~~~~~~~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:618:33: note: in expansion of macro ‘BT_LOG_ENABLED’
     #define BT_LOG_ENABLED_TRACE    BT_LOG_ENABLED(BT_LOG_TRACE)
                                     ^~~~~~~~~~~~~~
    /home/smarchi/src/babeltrace/src/logging/log.h:852:5: note: in expansion of macro ‘BT_LOG_ENABLED_TRACE’
     #if BT_LOG_ENABLED_TRACE
         ^~~~~~~~~~~~~~~~~~~~

This shows that while BT_LOGGING_LEVEL_* are not preprocessor macros,
they are being used in preprocessor conditions.  This makes the logging
macros (e.g. BT_LOGT) always defined to the concrete logging code, even
though the minimal compile-time log level should make them defined to
nothing.

Fix that by defining macros for logging levels (not exposed in the API),
which we access in log.c, the library's logging system.  The macros are
defined in a new file, logging-defs.h, because it's not possible to
include logging.h twice.

Now that the BT_LOG_ENABLED_TRACE macro is working as intended, it then
exposes this problem when building with the default setting of
BABELTRACE_MINIMAL_LOG_LEVEL=DEBUG.

    /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.y:52:0: error: "YYDEBUG" redefined [-Werror]
     # define YYDEBUG 0

    In file included from /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.y:44:0:
    /home/smarchi/src/babeltrace/src/plugins/ctf/common/metadata/parser.h:37:0: note: this is the location of the previous definition
     # define YYDEBUG 1

When building parser.c, parser.h is first included and defines YYDEBUG
with:

    #ifndef YYDEBUG
    # define YYDEBUG 1
    #endif

Then, we come with:

    #if BT_LOG_ENABLED_TRACE
    # define YYDEBUG 1
    # define YYFPRINTF(_stream, _fmt, args...) BT_LOGT(_fmt, ## args)
    #else
    # define YYDEBUG 0
    #endif

If we want to control YYDEBUG based on BT_LOG_ENABLED_TRACE, we need to
define that before including parser.h.  But that's not the end of our
troubles!  If YYDEBUG is defined as 0 when building parser.c, it will
not generated a definition for the variable yydebug.  Some other files
declare their own "extern int yydebug;" unconditionally and access it.
If yydebug has no definition, this results in undefined references to
yydebug.

To solve it, I want to make all files who use yydebug include some file
to obtain the yydebug declaration, which will be properly gated with the
`#if BT_LOG_ENABLED_TRACE` condition.  That new file is `parser-wrap.h`.
To make sure that nobody includes parser.h directly (which would lead to
yydebug always being declared for them), I've made it so you need to
define a special macro (ALLOW_INCLUDE_PARSER_H) to include it, which
only parser-wrap.h does.

Change-Id: I01ddaa3c8e4d3e5c119ecf221203023b678cc6fb
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2228
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
14 files changed:
include/Makefile.am
include/babeltrace2/babeltrace.h
include/babeltrace2/logging-defs.h [new file with mode: 0644]
include/babeltrace2/logging.h
src/logging/log.h
src/plugins/ctf/common/metadata/Makefile.am
src/plugins/ctf/common/metadata/decoder-packetized-file-stream-to-buf.c
src/plugins/ctf/common/metadata/decoder.c
src/plugins/ctf/common/metadata/lexer.l
src/plugins/ctf/common/metadata/parser-wrap.h [new file with mode: 0644]
src/plugins/ctf/common/metadata/parser.y
src/plugins/ctf/common/metadata/visitor-generate-ir.c
src/plugins/ctf/common/metadata/visitor-parent-links.c
src/plugins/ctf/common/metadata/visitor-semantic-validator.c

index 3ff5c579cacb8853064d7e787aad6a6699d578b7..98d0e31be94cb8f94df6b8c570dad39afc9c8e04 100644 (file)
@@ -9,6 +9,7 @@ babeltrace2include_HEADERS = \
        babeltrace2/integer-range-set-const.h \
        babeltrace2/integer-range-set.h \
        babeltrace2/logging.h \
+       babeltrace2/logging-defs.h \
        babeltrace2/property.h \
        babeltrace2/types.h \
        babeltrace2/util.h \
index ed17a864b964b95263f0db990c6f2e1847fedba1..5e099a593b978e0e662ecf0e21a04284c0151422 100644 (file)
 #undef __BT_IN_BABELTRACE_H
 #undef __BT_UPCAST
 #undef __BT_UPCAST_CONST
+#undef __BT_LOGGING_LEVEL_TRACE
+#undef __BT_LOGGING_LEVEL_DEBUG
+#undef __BT_LOGGING_LEVEL_INFO
+#undef __BT_LOGGING_LEVEL_WARNING
+#undef __BT_LOGGING_LEVEL_ERROR
+#undef __BT_LOGGING_LEVEL_FATAL
+#undef __BT_LOGGING_LEVEL_NONE
 
 #endif /* BABELTRACE2_BABELTRACE_H */
diff --git a/include/babeltrace2/logging-defs.h b/include/babeltrace2/logging-defs.h
new file mode 100644 (file)
index 0000000..b862889
--- /dev/null
@@ -0,0 +1,65 @@
+/*
+ * No include guards here: it is safe to include this file multiple
+ * times.
+ */
+
+/*
+ * Copyright (c) 2019 EfficiOS Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef __BT_IN_BABELTRACE_H
+# error "Please include <babeltrace2/babeltrace.h> instead."
+#endif
+
+/*
+ * This is NOT part of the API.
+ *
+ * These macros are used internally in preprocessor conditions to define
+ * logging macros.
+ */
+
+#ifndef __BT_LOGGING_LEVEL_TRACE
+# define __BT_LOGGING_LEVEL_TRACE      1
+#endif
+
+#ifndef __BT_LOGGING_LEVEL_DEBUG
+# define __BT_LOGGING_LEVEL_DEBUG      2
+#endif
+
+#ifndef __BT_LOGGING_LEVEL_INFO
+# define __BT_LOGGING_LEVEL_INFO       3
+#endif
+
+#ifndef __BT_LOGGING_LEVEL_WARNING
+# define __BT_LOGGING_LEVEL_WARNING    4
+#endif
+
+#ifndef __BT_LOGGING_LEVEL_ERROR
+# define __BT_LOGGING_LEVEL_ERROR      5
+#endif
+
+#ifndef __BT_LOGGING_LEVEL_FATAL
+# define __BT_LOGGING_LEVEL_FATAL      6
+#endif
+
+#ifndef __BT_LOGGING_LEVEL_NONE
+# define __BT_LOGGING_LEVEL_NONE       0xff
+#endif
index 37143e19b458149eccf86fae634a4784695d5bd8..8091ec436a2bc512f5eada01cb26c77a7ef7e905 100644 (file)
@@ -27,6 +27,8 @@
 # error "Please include <babeltrace2/babeltrace.h> instead."
 #endif
 
+#include <babeltrace2/logging-defs.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -62,19 +64,19 @@ current global log level and the minimal log level are not executed.
 */
 typedef enum bt_logging_level {
        /// Additional, low-level debugging context information.
-       BT_LOGGING_LEVEL_TRACE          = 1,
+       BT_LOGGING_LEVEL_TRACE          = __BT_LOGGING_LEVEL_TRACE,
 
        /**
        Debugging information, only useful when searching for the
        cause of a bug.
        */
-       BT_LOGGING_LEVEL_DEBUG          = 2,
+       BT_LOGGING_LEVEL_DEBUG          = __BT_LOGGING_LEVEL_DEBUG,
 
        /**
        Non-debugging information and failure to load optional
        subsystems.
        */
-       BT_LOGGING_LEVEL_INFO           = 3,
+       BT_LOGGING_LEVEL_INFO           = __BT_LOGGING_LEVEL_INFO,
 
        /**
        Errors caused by a bad usage of the library, that is, a
@@ -83,23 +85,23 @@ typedef enum bt_logging_level {
        The library's and object's states remain consistent when a
        warning is issued.
        */
-       BT_LOGGING_LEVEL_WARNING                = 4,
+       BT_LOGGING_LEVEL_WARNING        = __BT_LOGGING_LEVEL_WARNING,
 
        /**
        An important error from which the library cannot recover, but
        the executed stack of functions can still return cleanly.
        */
-       BT_LOGGING_LEVEL_ERROR          = 5,
+       BT_LOGGING_LEVEL_ERROR          = __BT_LOGGING_LEVEL_ERROR,
 
        /**
        The library cannot continue to work in this condition: it must
        terminate immediately, without even returning to the user's
        execution.
        */
-       BT_LOGGING_LEVEL_FATAL          = 6,
+       BT_LOGGING_LEVEL_FATAL          = __BT_LOGGING_LEVEL_FATAL,
 
        /// Logging is disabled.
-       BT_LOGGING_LEVEL_NONE           = 0xff,
+       BT_LOGGING_LEVEL_NONE           = __BT_LOGGING_LEVEL_NONE,
 } bt_logging_level;
 
 /**
index 993479c7fcfab48d9acb8138df5388c274d4f2f6..14062fa122ea2538eca594cb2ee4db4db5df3cfc 100644 (file)
 #include <string.h>
 #include <babeltrace2/babeltrace.h>
 
+/* Access private __BT_LOGGING_LEVEL_* macros. */
+#define __BT_IN_BABELTRACE_H
+#include <babeltrace2/logging-defs.h>
+#undef __BT_IN_BABELTRACE_H
+
 #include "common/macros.h"
 #include "common/assert.h"
 
  * should be empty or very small. Choosing a right log level is as important as
  * providing short and self descriptive log message.
  */
-#define BT_LOG_TRACE   BT_LOGGING_LEVEL_TRACE
-#define BT_LOG_DEBUG   BT_LOGGING_LEVEL_DEBUG
-#define BT_LOG_INFO    BT_LOGGING_LEVEL_INFO
-#define BT_LOG_WARNING BT_LOGGING_LEVEL_WARNING
-#define BT_LOG_ERROR   BT_LOGGING_LEVEL_ERROR
-#define BT_LOG_FATAL   BT_LOGGING_LEVEL_FATAL
-#define BT_LOG_NONE    BT_LOGGING_LEVEL_NONE
+#define BT_LOG_TRACE   __BT_LOGGING_LEVEL_TRACE
+#define BT_LOG_DEBUG   __BT_LOGGING_LEVEL_DEBUG
+#define BT_LOG_INFO    __BT_LOGGING_LEVEL_INFO
+#define BT_LOG_WARNING __BT_LOGGING_LEVEL_WARNING
+#define BT_LOG_ERROR   __BT_LOGGING_LEVEL_ERROR
+#define BT_LOG_FATAL   __BT_LOGGING_LEVEL_FATAL
+#define BT_LOG_NONE    __BT_LOGGING_LEVEL_NONE
 
 /* "Current" log level is a compile time check and has no runtime overhead. Log
  * level that is below current log level it said to be "disabled".
index aa3b4490081f20498d35750152cea21efddc1d9d..d37b3835a590d43f540108a07975b904ed1f1d62 100644 (file)
@@ -19,6 +19,7 @@ libctf_ast_la_SOURCES = \
        ast.h \
        objstack.h \
        parser.h \
+       parser-wrap.h \
        scanner.h \
        scanner-symbols.h \
        decoder.c \
index 07ddca858cf96c0d1958fbc6e3ab37f9befff84d..a2f197af7f9ad4320fee592279984f51d297f4fb 100644 (file)
@@ -36,9 +36,6 @@
 
 #define TSDL_MAGIC     0x75d11d57
 
-extern
-int yydebug;
-
 struct ctf_metadata_decoder {
        struct ctf_visitor_generate_ir *visitor;
        bt_uuid_t uuid;
index cfa8ec17b99871ec1df782f04b27a99ac5f7e53e..a62beebfcd99cce5dacafe88d96883c8492c14b4 100644 (file)
 #include "decoder.h"
 #include "scanner.h"
 #include "logging.h"
+#include "parser-wrap.h"
 
 #define TSDL_MAGIC     0x75d11d57
 
-extern
-int yydebug;
-
 struct ctf_metadata_decoder {
        struct ctf_scanner *scanner;
        GString *text;
@@ -271,9 +269,11 @@ enum ctf_metadata_decoder_status ctf_metadata_decoder_append_content(
                }
        }
 
+#if YYDEBUG
        if (BT_LOG_ON_TRACE) {
                yydebug = 1;
        }
+#endif
 
        /* Save the file's position: we'll seek back to append the plain text */
        BT_ASSERT(fp);
@@ -341,7 +341,9 @@ enum ctf_metadata_decoder_status ctf_metadata_decoder_append_content(
        }
 
 end:
+#if YYDEBUG
        yydebug = 0;
+#endif
 
        if (fp && close_fp) {
                if (fclose(fp)) {
index b88dec879e326ec883814f31dc7e246426badba8..4f268dc69994a6dc2148cb4b91d13d0dd605863a 100644 (file)
@@ -32,7 +32,7 @@
 #include <stdio.h>
 #include <ctype.h>
 #include "scanner.h"
-#include "parser.h"
+#include "parser-wrap.h"
 #include "ast.h"
 
 #define YY_FATAL_ERROR(_msg)   BT_LOGF_STR(_msg)
diff --git a/src/plugins/ctf/common/metadata/parser-wrap.h b/src/plugins/ctf/common/metadata/parser-wrap.h
new file mode 100644 (file)
index 0000000..7eda762
--- /dev/null
@@ -0,0 +1,44 @@
+#ifndef BABELTRACE_PLUGINS_CTF_COMMON_METADATA_PARSER_WRAP_H
+#define BABELTRACE_PLUGINS_CTF_COMMON_METADATA_PARSER_WRAP_H
+
+/*
+ * Copyright 2019 EfficiOS Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+/*
+ * Small wrapper around the bison-generated parser.h to conditionally define
+ * YYDEBUG (and therefore the yydebug declaration).
+ */
+
+#include "logging/log.h"
+
+#if BT_LOG_ENABLED_TRACE
+# define YYDEBUG 1
+# define YYFPRINTF(_stream, _fmt, args...) BT_LOGT(_fmt, ## args)
+#else
+# define YYDEBUG 0
+#endif
+
+#define ALLOW_INCLUDE_PARSER_H
+#include "parser.h"
+#undef ALLOW_INCLUDE_PARSER_H
+
+#endif
index ef89fcf8bfeb714ce750cd982857cc5305f4425a..b270a83c5001b89d5b4398398420a55587eed259 100644 (file)
 #include "common/list.h"
 #include "common/assert.h"
 #include "scanner.h"
-#include "parser.h"
 #include "ast.h"
 #include "objstack.h"
 
-#if BT_LOG_ENABLED_TRACE
-# define YYDEBUG 1
-# define YYFPRINTF(_stream, _fmt, args...) BT_LOGT(_fmt, ## args)
-#else
-# define YYDEBUG 0
-#endif
+#include "parser-wrap.h"
 
 /* Join two lists, put "add" at the end of "head".  */
 static inline void
@@ -1044,6 +1038,17 @@ void ctf_scanner_free(struct ctf_scanner *scanner)
 
 %}
 
+/*
+ * This ends up in parser.h and makes sure those who want to include it pass
+ * through parser-wrap.h.
+ */
+%code requires {
+#ifndef ALLOW_INCLUDE_PARSER_H
+# error "Don't include parser.h directly, include parser-wrap.h instead."
+#endif
+}
+
+
 %define api.pure
        /* %locations */
 %error-verbose
index e702a98018dac93109c1a3484515f7917e79d587..99d6ae4c83de8ef320a4413a4cce74b939a445de 100644 (file)
@@ -49,7 +49,6 @@
 
 #include "logging.h"
 #include "scanner.h"
-#include "parser.h"
 #include "ast.h"
 #include "decoder.h"
 #include "ctf-meta.h"
index af549b8a1afb3f345dceb2dbf98d7a7ff751b5de..6300e3f31628da53d8f44e1a7af151ecb49c5563 100644 (file)
@@ -40,7 +40,6 @@
 #include "common/macros.h"
 #include "common/list.h"
 #include "scanner.h"
-#include "parser.h"
 #include "ast.h"
 #include "logging.h"
 
index 32cff557037aaf2d823411067884785ba642aeec..d22e4eee02dd30803ebf92c22bfab0c7530cf9a0 100644 (file)
@@ -39,7 +39,6 @@
 #include <errno.h>
 #include "common/list.h"
 #include "scanner.h"
-#include "parser.h"
 #include "ast.h"
 #include "logging.h"
 
This page took 0.032986 seconds and 4 git commands to generate.