From 9103e903a89377e9cfad13905d4f4b650aecd061 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 21 Oct 2019 13:59:50 -0400 Subject: [PATCH] Fix: define macros for logging levels MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/2228 Tested-by: jenkins Reviewed-by: Francis Deslauriers --- include/Makefile.am | 1 + include/babeltrace2/babeltrace.h | 7 ++ include/babeltrace2/logging-defs.h | 65 +++++++++++++++++++ include/babeltrace2/logging.h | 16 +++-- src/logging/log.h | 19 ++++-- src/plugins/ctf/common/metadata/Makefile.am | 1 + .../decoder-packetized-file-stream-to-buf.c | 3 - src/plugins/ctf/common/metadata/decoder.c | 8 ++- src/plugins/ctf/common/metadata/lexer.l | 2 +- src/plugins/ctf/common/metadata/parser-wrap.h | 44 +++++++++++++ src/plugins/ctf/common/metadata/parser.y | 19 ++++-- .../ctf/common/metadata/visitor-generate-ir.c | 1 - .../common/metadata/visitor-parent-links.c | 1 - .../metadata/visitor-semantic-validator.c | 1 - 14 files changed, 157 insertions(+), 31 deletions(-) create mode 100644 include/babeltrace2/logging-defs.h create mode 100644 src/plugins/ctf/common/metadata/parser-wrap.h diff --git a/include/Makefile.am b/include/Makefile.am index 3ff5c579..98d0e31b 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -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 \ diff --git a/include/babeltrace2/babeltrace.h b/include/babeltrace2/babeltrace.h index ed17a864..5e099a59 100644 --- a/include/babeltrace2/babeltrace.h +++ b/include/babeltrace2/babeltrace.h @@ -173,5 +173,12 @@ #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 index 00000000..b8628898 --- /dev/null +++ b/include/babeltrace2/logging-defs.h @@ -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 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 diff --git a/include/babeltrace2/logging.h b/include/babeltrace2/logging.h index 37143e19..8091ec43 100644 --- a/include/babeltrace2/logging.h +++ b/include/babeltrace2/logging.h @@ -27,6 +27,8 @@ # error "Please include instead." #endif +#include + #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; /** diff --git a/src/logging/log.h b/src/logging/log.h index 993479c7..14062fa1 100644 --- a/src/logging/log.h +++ b/src/logging/log.h @@ -15,6 +15,11 @@ #include #include +/* Access private __BT_LOGGING_LEVEL_* macros. */ +#define __BT_IN_BABELTRACE_H +#include +#undef __BT_IN_BABELTRACE_H + #include "common/macros.h" #include "common/assert.h" @@ -55,13 +60,13 @@ * 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". diff --git a/src/plugins/ctf/common/metadata/Makefile.am b/src/plugins/ctf/common/metadata/Makefile.am index aa3b4490..d37b3835 100644 --- a/src/plugins/ctf/common/metadata/Makefile.am +++ b/src/plugins/ctf/common/metadata/Makefile.am @@ -19,6 +19,7 @@ libctf_ast_la_SOURCES = \ ast.h \ objstack.h \ parser.h \ + parser-wrap.h \ scanner.h \ scanner-symbols.h \ decoder.c \ diff --git a/src/plugins/ctf/common/metadata/decoder-packetized-file-stream-to-buf.c b/src/plugins/ctf/common/metadata/decoder-packetized-file-stream-to-buf.c index 07ddca85..a2f197af 100644 --- a/src/plugins/ctf/common/metadata/decoder-packetized-file-stream-to-buf.c +++ b/src/plugins/ctf/common/metadata/decoder-packetized-file-stream-to-buf.c @@ -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; diff --git a/src/plugins/ctf/common/metadata/decoder.c b/src/plugins/ctf/common/metadata/decoder.c index cfa8ec17..a62beebf 100644 --- a/src/plugins/ctf/common/metadata/decoder.c +++ b/src/plugins/ctf/common/metadata/decoder.c @@ -33,12 +33,10 @@ #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)) { diff --git a/src/plugins/ctf/common/metadata/lexer.l b/src/plugins/ctf/common/metadata/lexer.l index b88dec87..4f268dc6 100644 --- a/src/plugins/ctf/common/metadata/lexer.l +++ b/src/plugins/ctf/common/metadata/lexer.l @@ -32,7 +32,7 @@ #include #include #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 index 00000000..7eda7621 --- /dev/null +++ b/src/plugins/ctf/common/metadata/parser-wrap.h @@ -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 diff --git a/src/plugins/ctf/common/metadata/parser.y b/src/plugins/ctf/common/metadata/parser.y index ef89fcf8..b270a83c 100644 --- a/src/plugins/ctf/common/metadata/parser.y +++ b/src/plugins/ctf/common/metadata/parser.y @@ -41,16 +41,10 @@ #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 diff --git a/src/plugins/ctf/common/metadata/visitor-generate-ir.c b/src/plugins/ctf/common/metadata/visitor-generate-ir.c index e702a980..99d6ae4c 100644 --- a/src/plugins/ctf/common/metadata/visitor-generate-ir.c +++ b/src/plugins/ctf/common/metadata/visitor-generate-ir.c @@ -49,7 +49,6 @@ #include "logging.h" #include "scanner.h" -#include "parser.h" #include "ast.h" #include "decoder.h" #include "ctf-meta.h" diff --git a/src/plugins/ctf/common/metadata/visitor-parent-links.c b/src/plugins/ctf/common/metadata/visitor-parent-links.c index af549b8a..6300e3f3 100644 --- a/src/plugins/ctf/common/metadata/visitor-parent-links.c +++ b/src/plugins/ctf/common/metadata/visitor-parent-links.c @@ -40,7 +40,6 @@ #include "common/macros.h" #include "common/list.h" #include "scanner.h" -#include "parser.h" #include "ast.h" #include "logging.h" diff --git a/src/plugins/ctf/common/metadata/visitor-semantic-validator.c b/src/plugins/ctf/common/metadata/visitor-semantic-validator.c index 32cff557..d22e4eee 100644 --- a/src/plugins/ctf/common/metadata/visitor-semantic-validator.c +++ b/src/plugins/ctf/common/metadata/visitor-semantic-validator.c @@ -39,7 +39,6 @@ #include #include "common/list.h" #include "scanner.h" -#include "parser.h" #include "ast.h" #include "logging.h" -- 2.34.1