Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/10116 )
Change subject: logging: Fix logging level all ...................................................................... Patch Set 1: Code-Review-1 (2 comments) https://gerrit.osmocom.org/#/c/10116/1/src/logging.c File src/logging.c: https://gerrit.osmocom.org/#/c/10116/1/src/logging.c@721 PS1, Line 721: for (i = 0; i < osmo_log_info->num_cat; i++) { > I also thought about what you mentioned. I actually thought about > calling it "logging level unset <loglevel>", which would basically > apply to all unset categories. It would be the same as "all", but > without forcing reset of the categories. I think it would be even more flexible to have a separate command: logging level unset-all which would do the only thing - resetting all categories, and nothing else. > I even though about changing log_set_log_level() to contain a "bool reset" > param, but since it's actually a public API we cannot do that. In any case, it's strange that so simple one-line function, that only changes the publicly available member of a logging target, is a part of the public API... > Looking at current users of log_set_log_level() through all osmocom projects, > I think in general the desired behaviour is to reset the categories. > I would then have another function: > log_set_log_level2(struct log_target *target, int log_level, bool reset), > and use the reset=false only in the case of: > "logging level unset <loglevel>" > in VTY. Oh, let's please avoid introducing foo2, bar3, etc., especially in the cases when it isn't that important and not that hard to give a better name... Instead, let's have a separate function (UNIX way) just to reset the logging categories to 'unset'? It could be done in a separate change I think. We should also ask both Neels and Harald, as Neels has been working some VTY changes, and Harald has lots of experience in general... https://gerrit.osmocom.org/#/c/10116/1/src/vty/logging_vty.c File src/vty/logging_vty.c: https://gerrit.osmocom.org/#/c/10116/1/src/vty/logging_vty.c@a283 PS1, Line 283: > I would expect "everything" to not be available anymore after this patch, so > parsing should fail bef […] Still not sure about this, sorry. We are going to drop some deprecated functionality in core by hiding this as a part of flabbily related change... That can be also done and discussed in a separate change. -- To view, visit https://gerrit.osmocom.org/10116 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0f50ad8d6fd038398f7d751287417505c8dcdeff Gerrit-Change-Number: 10116 Gerrit-PatchSet: 1 Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilira...@gmail.com> Gerrit-Comment-Date: Mon, 23 Jul 2018 17:24:08 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes