libosmocore[master]: vty: fix "logging level ... everything" option

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Mon Mar 13 03:06:59 UTC 2017


Patch Set 10: Code-Review-1

(5 comments)

As said before, I think we cannot fix 'everything' because the cat is out of the bag. As much as I'd like to fix 'everything', doing so might break working setups out there that have 'everything' in their config without being aware of it. We may explode their logs without warning; I believe that unfortunately all we can do is deprecate 'everything'.

Also, with the unrelated cosmetic changes in this patch I fail to see the actual fix...

And let's discuss changing the public API, which, as much as I'd like to have them because they make sense, might introduce build problems.

https://gerrit.osmocom.org/#/c/1582/10/include/osmocom/core/logging.h
File include/osmocom/core/logging.h:

Line 329: 
I don't think we want to remove API that has once been published for backwards compat ... how sure are you that no-one anywhere is using this?


https://gerrit.osmocom.org/#/c/1582/10/include/osmocom/vty/logging.h
File include/osmocom/vty/logging.h:

Line 7: void logging_vty_add_cmds();
this is also a change of the public API, right? How sure are you that no-one anywhere is calling logging_vty_add_cmds() with an arg outside of libosmocore?

Also: unrelated to fixing 'all everything'


https://gerrit.osmocom.org/#/c/1582/10/src/logging.c
File src/logging.c:

Line 69: 	{ LOGL_DEBUG,	"EVERYTHING" }, /* FIXME: remove when 'everything' is no longer necessary */
the same value twice??


Line 74: 	{ 0,		"DEBUG" }, /* FIXME: remove when 'everything' is no longer necessary */
LOGL_DEBUG and 0 both amount to "DEBUG"?

Both directions of using this value_string[] are ambiguous. LOGL_DEBUG -> { "DEBUG", "EVERYTHING" } and "DEBUG" -> { LOGL_DEBUG, 0 }.


Line 364: static inline bool should_log_to_target(struct log_target *tar, int subsys,
I like this but is unrelated


-- 
To view, visit https://gerrit.osmocom.org/1582
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib51987fb2f64a70fca130f3799dc8fd71cc7085c
Gerrit-PatchSet: 10
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list