[PATCH] osmo-trx[master]: Logger: Use libosmocore logging system

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Tue Feb 20 19:55:03 UTC 2018


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 (#4).

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/4

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: 4
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder


More information about the gerrit-log mailing list