Cellular Network Infrastructure » Core Network (CN) » OsmoSGSN

To ensure proper default loglevel I think it makes sense to print a message as a reminder. Maybe later when we fixed the problem in every osmocom project, then we can migrate to an OSMO_ASSERT()

https://gerrit.osmocom.org/c/libosmocore/+/16832 logging.c: Add valuestrings for loglevels
https://gerrit.osmocom.org/c/libosmocore/+/16833 application.c: check default loglevels on startup

I think it makes much more sense to test this as a unit test for apps where we really want to avoid DEBUG being default. I (or someone else) may be writing an app which uses DEBUG by default for whatever reason, and we must not reject that.

I would think it's sufficient to go through all applications (it's not that many, after all) and ensure that the current code doesn't have any compiled-in default log level below NOTICE or INFO. In the future we should pay attention during code review to prevent new such bugs from being introduced. I would expect they date back to our pre-gerrit development days.

This way thre's no need for enforcement in libosmocore, which may create problems as pau indicates.

I have now checked the applications. LOGL_NOTICE seems to be the standard everywhere. I have found some odd loglevels in osmocom-bb and osmo-bsc which i have corrected. There is also some old stuff bs_11_config and openBSC which I did not touch.

Looking at the commits related to this ticket, I noticed that we're basically changing all logging categories to use LOGL_NOTICE. In the end I see an array of struct log_info_cat where every entry has the same line: '.loglevel = LOGL_NOTICE'. This looks odd and cumbersome, so I've got an idea.

LOGL_DEBUG has the lowest value, and it starts from 1:

/*! different log levels */
#define LOGL_DEBUG      1       /*!< debugging information */
#define LOGL_INFO       3       /*!< general information */
#define LOGL_NOTICE     5       /*!< abnormal/unexpected condition */
#define LOGL_ERROR      7       /*!< error condition, requires user action */
#define LOGL_FATAL      8       /*!< fatal, program aborted */

this means that we can omit those repeating lines, and leave it up to log_init() to initialize the default logging level:

diff --git a/src/logging.c b/src/logging.c
index 4d6224d5..60719d50 100644
--- a/src/logging.c
+++ b/src/logging.c
@@ -1056,6 +1056,8 @@ int log_init(const struct log_info *inf, void *ctx)
                for (i = 0; i < inf->num_cat; i++) {
                        memcpy((struct log_info_cat *) &osmo_log_info->cat[i],
                               &inf->cat[i], sizeof(struct log_info_cat));
+                       if (!inf->cat[i].loglevel)
+                               !inf->cat[i].loglevel = LOGL_NOTICE;
                }
        }

I like the idea, lets integrate that.