Hello Jenkins Builder, I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/6620 to look at the new patch set (#3). Logger: Use libosmocore logging system We still need an intermediate class Logger due to osmo-trx being multi-threaded and requiring to have a lock to use libosmocore, which is not thread safe. Change-Id: I30baac89f53e927f8699d0586b43cccf88ecd493 --- M CommonLibs/Logger.cpp M CommonLibs/Logger.h M CommonLibs/Makefile.am M Transceiver52M/osmo-trx.cpp M tests/CommonLibs/LogTest.cpp M tests/CommonLibs/LogTest.err M tests/CommonLibs/LogTest.ok M tests/CommonLibs/Makefile.am 8 files changed, 64 insertions(+), 187 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/20/6620/3 diff --git a/CommonLibs/Logger.cpp b/CommonLibs/Logger.cpp index 4c2a2d3..d6125a7 100644 --- a/CommonLibs/Logger.cpp +++ b/CommonLibs/Logger.cpp @@ -39,55 +39,6 @@ Mutex gLogToLock; -// Global log level threshold: -int config_log_level; - -/** Names of the logging levels. */ -const char *levelNames[] = { - "EMERG", "ALERT", "CRIT", "ERR", "WARNING", "NOTICE", "INFO", "DEBUG" -}; -int numLevels = 8; - - -int levelStringToInt(const string& name) -{ - // Reverse search, since the numerically larger levels are more common. - for (int i=numLevels-1; i>=0; i--) { - if (name == levelNames[i]) return i; - } - - // Common substitutions. - if (name=="INFORMATION") return 6; - if (name=="WARN") return 4; - if (name=="ERROR") return 3; - if (name=="CRITICAL") return 2; - if (name=="EMERGENCY") return 0; - - // Unknown level. - return -1; -} - -static std::string format(const char *fmt, ...) -{ - va_list ap; - char buf[300]; - va_start(ap,fmt); - int n = vsnprintf(buf,300,fmt,ap); - va_end(ap); - if (n >= (300-4)) { strcpy(&buf[(300-4)],"..."); } - return std::string(buf); -} - -const std::string timestr() -{ - struct timeval tv; - struct tm tm; - gettimeofday(&tv,NULL); - localtime_r(&tv.tv_sec,&tm); - unsigned tenths = tv.tv_usec / 100000; // Rounding down is ok. - return format(" %02d:%02d:%02d.%1d",tm.tm_hour,tm.tm_min,tm.tm_sec,tenths); -} - std::ostream& operator<<(std::ostream& os, std::ostringstream& ss) { return os << ss.str(); @@ -95,34 +46,18 @@ Log::~Log() { - // Anything at or above LOG_CRIT is an "alarm". - if (mPriority <= LOG_ERR) { - cerr << mStream.str() << endl; - } - int mlen = mStream.str().size(); int neednl = (mlen==0 || mStream.str()[mlen-1] != '\n'); + const char *fmt = neednl ? "%s\n" : "%s"; ScopedLock lock(gLogToLock); // The COUT() macro prevents messages from stomping each other but adds uninteresting thread numbers, // so just use std::cout. - std::cout << mStream.str(); - if (neednl) std::cout<<"\n"; + LOGP(mCategory, mPriority, fmt, mStream.str().c_str()); } ostringstream& Log::get() { - assert(mPriority<numLevels); - mStream << levelNames[mPriority] << ' '; return mStream; -} - - - -void gLogInit(const char* level, char *fn) -{ - // Set the level if one has been specified. - if (level) - config_log_level = levelStringToInt(level); } // vim: ts=4 sw=4 diff --git a/CommonLibs/Logger.h b/CommonLibs/Logger.h index a8fe44d..b532e8b 100644 --- a/CommonLibs/Logger.h +++ b/CommonLibs/Logger.h @@ -23,12 +23,6 @@ */ -// (pat) WARNING is stupidly defined in /usr/local/include/osipparser2/osip_const.h. -// This must be outside the #ifndef LOGGER_H to fix it as long as Logger.h included after the above file. -#ifdef WARNING -#undef WARNING -#endif - #ifndef LOGGER_H #define LOGGER_H @@ -37,30 +31,27 @@ #include <sstream> #include <string> -extern int config_log_level; +extern "C" { +#include <osmocom/core/logging.h> +#include "debug.h" +} -#define LOG_EMERG 0 /* system is unusable */ -#define LOG_ALERT 1 /* action must be taken immediately */ -#define LOG_CRIT 2 /* critical conditions */ -#define LOG_ERR 3 /* error conditions */ -#define LOG_WARNING 4 /* warning conditions */ -#define LOG_NOTICE 5 /* normal but significant condition */ -#define LOG_INFO 6 /* informational */ -#define LOG_DEBUG 7 /* debug-level messages */ - -#define _LOG(level) \ - Log(LOG_##level).get() << pthread_self() \ - << timestr() << " " __FILE__ ":" << __LINE__ << ":" << __FUNCTION__ << ": " - -#define IS_LOG_LEVEL(wLevel) (config_log_level>=LOG_##wLevel) - -#ifdef NDEBUG -#define LOG(wLevel) \ - if (LOG_##wLevel!=LOG_DEBUG && IS_LOG_LEVEL(wLevel)) _LOG(wLevel) -#else -#define LOG(wLevel) \ - if (IS_LOG_LEVEL(wLevel)) _LOG(wLevel) +/* Translation for old log statements */ +#ifndef LOGL_ALERT +#define LOGL_ALERT LOGL_FATAL #endif +#ifndef LOGL_ERR +#define LOGL_ERR LOGL_ERROR +#endif +#ifndef LOGL_WARNING +#define LOGL_WARNING LOGL_NOTICE +#endif + +#define LOG(level) \ + Log(DTRX, LOGL_##level).get() << "[tid=" << pthread_self() << "] " + +#define LOGC(category, level) \ + Log(category, LOGL_##level).get() << "[tid=" << pthread_self() << "] " /** A C++ stream-based thread-safe logger. @@ -73,13 +64,14 @@ protected: - std::ostringstream mStream; ///< This is where we buffer up the log entry. - int mPriority; ///< Priority of current report. + std::ostringstream mStream; ///< This is where we buffer up the log entry. + int mCategory; ///< Priority of current report. + int mPriority; ///< Category of current report. public: - Log(int wPriority) - :mPriority(wPriority) + Log(int wCategory, int wPriority) + : mCategory(wCategory), mPriority(wPriority) { } // Most of the work is in the destructor. @@ -89,15 +81,7 @@ std::ostringstream& get(); }; -const std::string timestr(); // A timestamp to print in messages. std::ostream& operator<<(std::ostream& os, std::ostringstream& ss); - -/**@ Global control and initialization of the logging system. */ -//@{ -/** Initialize the global logging system. */ -void gLogInit(const char* level=NULL, char* fn=NULL); -//@} - #endif diff --git a/CommonLibs/Makefile.am b/CommonLibs/Makefile.am index 843bc38..5d116f9 100644 --- a/CommonLibs/Makefile.am +++ b/CommonLibs/Makefile.am @@ -22,7 +22,7 @@ include $(top_srcdir)/Makefile.common AM_CPPFLAGS = $(STD_DEFINES_AND_INCLUDES) -AM_CXXFLAGS = -Wall -O3 -g -lpthread +AM_CXXFLAGS = -Wall -O3 -g -lpthread $(LIBOSMOCORE_CFLAGS) $(LIBOSMOCTRL_CFLAGS) $(LIBOSMOVTY_CFLAGS) AM_CFLAGS = $(LIBOSMOCORE_CFLAGS) $(LIBOSMOCTRL_CFLAGS) $(LIBOSMOVTY_CFLAGS) EXTRA_DIST = \ diff --git a/Transceiver52M/osmo-trx.cpp b/Transceiver52M/osmo-trx.cpp index 4fff8ad..7726ca0 100644 --- a/Transceiver52M/osmo-trx.cpp +++ b/Transceiver52M/osmo-trx.cpp @@ -79,7 +79,6 @@ #define DEFAULT_CONFIG_FILE "/etc/osmocom/osmo-trx.cfg" struct trx_config { - std::string log_level; std::string local_addr; std::string remote_addr; std::string dev_args; @@ -156,7 +155,6 @@ std::ostringstream ost(""); ost << "Config Settings" << std::endl; - ost << " Log Level............... " << config->log_level << std::endl; ost << " Device args............. " << config->dev_args << std::endl; ost << " TRX Base Port........... " << config->port << std::endl; ost << " TRX Address............. " << config->local_addr << std::endl; @@ -311,7 +309,6 @@ fprintf(stdout, "Options:\n" " -h This text\n" " -a UHD device args\n" - " -l Logging level (%s)\n" " -i IP address of GSM core\n" " -j IP address of osmo-trx\n" " -p Base port number\n" @@ -330,8 +327,8 @@ " -S Swap channels (UmTRX only)\n" " -t SCHED_RR real-time priority (1..32)\n" " -y comma-delimited list of Tx paths (num elements matches -c)\n" - " -z comma-delimited list of Rx paths (num elements matches -c)\n", - "EMERG, ALERT, CRT, ERR, WARNING, NOTICE, INFO, DEBUG"); + " -z comma-delimited list of Rx paths (num elements matches -c)\n" + ); } static void handle_options(int argc, char **argv, struct trx_config *config) @@ -339,7 +336,6 @@ int option; bool tx_path_set = false, rx_path_set = false; - config->log_level = "NOTICE"; config->local_addr = DEFAULT_TRX_IP; config->remote_addr = DEFAULT_TRX_IP; config->config_file = DEFAULT_CONFIG_FILE; @@ -361,7 +357,7 @@ config->tx_paths = std::vector<std::string>(DEFAULT_CHANS, ""); config->rx_paths = std::vector<std::string>(DEFAULT_CHANS, ""); - while ((option = getopt(argc, argv, "ha:l:i:j:p:c:dmxgfo:s:b:r:A:R:Set:y:z:C:")) != -1) { + while ((option = getopt(argc, argv, "ha:i:j:p:c:dmxgfo:s:b:r:A:R:Set:y:z:C:")) != -1) { switch (option) { case 'h': print_help(); @@ -369,9 +365,6 @@ break; case 'a': config->dev_args = optarg; - break; - case 'l': - config->log_level = optarg; break; case 'i': config->remote_addr = optarg; @@ -593,8 +586,6 @@ std::cerr << "Config: Database failure - exiting" << std::endl; return EXIT_FAILURE; } - - gLogInit(config.log_level.c_str()); srandom(time(NULL)); diff --git a/tests/CommonLibs/LogTest.cpp b/tests/CommonLibs/LogTest.cpp index 707e26c..b8677e6 100644 --- a/tests/CommonLibs/LogTest.cpp +++ b/tests/CommonLibs/LogTest.cpp @@ -28,21 +28,37 @@ #include <iterator> #include "Logger.h" +extern "C" { +#include <osmocom/core/application.h> +#include <osmocom/core/utils.h> +#include "debug.h" +} + +#define MYCAT 0 int main(int argc, char *argv[]) { - gLogInit("NOTICE"); + struct log_info_cat categories[1]; + struct log_info linfo; + categories[MYCAT] = { + "MYCAT", + NULL, + "Whatever", + LOGL_NOTICE, + 1, + }; + linfo.cat = categories; + linfo.num_cat = ARRAY_SIZE(categories); - Log(LOG_EMERG).get() << " testing the logger."; - Log(LOG_ALERT).get() << " testing the logger."; - Log(LOG_CRIT).get() << " testing the logger."; - Log(LOG_ERR).get() << " testing the logger."; - Log(LOG_WARNING).get() << " testing the logger."; - Log(LOG_NOTICE).get() << " testing the logger."; - Log(LOG_INFO).get() << " testing the logger."; - Log(LOG_DEBUG).get() << " testing the logger."; - std::cout << "----------- generating 20 alarms ----------" << std::endl; - for (int i = 0 ; i < 20 ; ++i) { - Log(LOG_ALERT).get() << i; - } + osmo_init_logging(&linfo); + + log_set_use_color(osmo_stderr_target, 0); + log_set_print_filename(osmo_stderr_target, 0); + log_set_print_level(osmo_stderr_target, 1); + + Log(MYCAT, LOGL_FATAL).get() << "testing the logger."; + Log(MYCAT, LOGL_ERROR).get() << "testing the logger."; + Log(MYCAT, LOGL_NOTICE).get() << "testing the logger."; + Log(MYCAT, LOGL_INFO).get() << "testing the logger."; + Log(MYCAT, LOGL_DEBUG).get() << "testing the logger."; } diff --git a/tests/CommonLibs/LogTest.err b/tests/CommonLibs/LogTest.err index 718ff40..153e850 100644 --- a/tests/CommonLibs/LogTest.err +++ b/tests/CommonLibs/LogTest.err @@ -1,24 +1,3 @@ -EMERG testing the logger. -ALERT testing the logger. -CRIT testing the logger. -ERR testing the logger. -ALERT 0 -ALERT 1 -ALERT 2 -ALERT 3 -ALERT 4 -ALERT 5 -ALERT 6 -ALERT 7 -ALERT 8 -ALERT 9 -ALERT 10 -ALERT 11 -ALERT 12 -ALERT 13 -ALERT 14 -ALERT 15 -ALERT 16 -ALERT 17 -ALERT 18 -ALERT 19 +FATAL testing the logger. +ERROR testing the logger. +NOTICE testing the logger. diff --git a/tests/CommonLibs/LogTest.ok b/tests/CommonLibs/LogTest.ok index ac60314..e69de29 100644 --- a/tests/CommonLibs/LogTest.ok +++ b/tests/CommonLibs/LogTest.ok @@ -1,29 +0,0 @@ -EMERG testing the logger. -ALERT testing the logger. -CRIT testing the logger. -ERR testing the logger. -WARNING testing the logger. -NOTICE testing the logger. -INFO testing the logger. -DEBUG testing the logger. ------------ generating 20 alarms ---------- -ALERT 0 -ALERT 1 -ALERT 2 -ALERT 3 -ALERT 4 -ALERT 5 -ALERT 6 -ALERT 7 -ALERT 8 -ALERT 9 -ALERT 10 -ALERT 11 -ALERT 12 -ALERT 13 -ALERT 14 -ALERT 15 -ALERT 16 -ALERT 17 -ALERT 18 -ALERT 19 diff --git a/tests/CommonLibs/Makefile.am b/tests/CommonLibs/Makefile.am index 6bd1852..721c9a2 100644 --- a/tests/CommonLibs/Makefile.am +++ b/tests/CommonLibs/Makefile.am @@ -1,6 +1,7 @@ include $(top_srcdir)/Makefile.common -AM_CPPFLAGS = -Wall -I$(top_srcdir)/CommonLibs $(STD_DEFINES_AND_INCLUDES) -g +AM_CPPFLAGS = -Wall -I$(top_srcdir)/CommonLibs $(STD_DEFINES_AND_INCLUDES) $(LIBOSMOCORE_CFLAGS) $(LIBOSMOCTRL_CFLAGS) $(LIBOSMOVTY_CFLAGS) -g +AM_LDFLAGS = $(LIBOSMOCORE_LIBS) $(LIBOSMOCTRL_LIBS) $(LIBOSMOVTY_LIBS) EXTRA_DIST = BitVectorTest.ok \ PRBSTest.ok \ -- To view, visit https://gerrit.osmocom.org/6620 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30baac89f53e927f8699d0586b43cccf88ecd493 Gerrit-PatchSet: 3 Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder