Attention is currently required from: Hoernchen, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30633 )
Change subject: logging: add log level cache
......................................................................
Patch Set 9:
(2 comments)
File src/logging.c:
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/9a7f5493_3264212b
PS6, Line 107: memset(tmp_level, 100, osmo_log_info->num_cat);
> Done
NOT DONE YET
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/487fae7c_0b8878ed
PS6, Line 156: return (level < log_level_lookup_cache[mapped_subsys]) ? false : true;
> This is precisely the inverted "when not to log" approach used by the existing check and I am reusin […]
Just a hint, leave it as you wish.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30633
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
Gerrit-Change-Number: 30633
Gerrit-PatchSet: 9
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 19 Dec 2022 18:27:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hoernchen <ewild(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/30633
to look at the new patch set (#9).
Change subject: logging: add log level cache
......................................................................
logging: add log level cache
This ensures multithreaded logging attempts, in particular ones that do
nothing, do not hold the lock just for checking the level, which
interferes with other logging attempts.
Closes: OS#5818
Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
---
M include/osmocom/core/logging.h
M src/logging.c
M src/vty/logging_vty.c
M tests/ctrl/ctrl_test.c
4 files changed, 124 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/30633/9
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30633
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
Gerrit-Change-Number: 30633
Gerrit-PatchSet: 9
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/30633
to look at the new patch set (#8).
Change subject: logging: add log level cache
......................................................................
logging: add log level cache
This ensures multithreaded logging attempts, in particular ones that do
nothing, do not hold the lock just for checking the level, which
interferes with other logging attempts.
Closes: OS#5818
Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
---
M include/osmocom/core/logging.h
M src/logging.c
M src/vty/logging_vty.c
M tests/ctrl/ctrl_test.c
4 files changed, 124 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/30633/8
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30633
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
Gerrit-Change-Number: 30633
Gerrit-PatchSet: 8
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, pespin.
Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30633 )
Change subject: logging: add log level cache
......................................................................
Patch Set 7:
(5 comments)
Patchset:
PS5:
> thanks for trying and explaining. […]
I would rather keep all those functions within the current !embedded define area instead of deliberately moving those function outside just to get the optimizer to remove those.. They should not be used or exist for embedded targets anyway, so the current way is safer, because embedded builds would error due to dangling symbols.
File src/logging.c:
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/15b803f1_c5f5eebe
PS6, Line 107: memset(tmp_level, 100, osmo_log_info->num_cat);
> It would be great to have this 100 and 99 which I see used below defined with a macro, holding a des […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/1a1e2053_55b31dd9
PS6, Line 134: struct log_category tmp = { 99, 0 };
> using { .field_a = 99, . […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/8cb45442_fc7ceee4
PS6, Line 145: log_level_lookup_cache[mapped_subsys] = tmp.enabled ? tmp.loglevel : 99;
> Please put this 99 in a define. […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/32e1dd7b_4e75c1dc
PS6, Line 156: return (level < log_level_lookup_cache[mapped_subsys]) ? false : true;
> "return (log_level_lookup_cache[mapped_subsys] >= level)" seems a lot shorter :)
This is precisely the inverted "when not to log" approach used by the existing check and I am reusing the style that is already present in the code because I hoped that by doing that I would not get into these kind ofs discussions.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30633
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
Gerrit-Change-Number: 30633
Gerrit-PatchSet: 7
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 19 Dec 2022 17:28:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hoernchen <ewild(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: Hoernchen, laforge.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/30633
to look at the new patch set (#7).
Change subject: logging: add log level cache
......................................................................
logging: add log level cache
This ensures multithreaded logging attempts, in particular ones that do
nothing, do not hold the lock just for checking the level, which
interferes with other logging attempts.
Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
---
M include/osmocom/core/logging.h
M src/logging.c
M src/vty/logging_vty.c
M tests/ctrl/ctrl_test.c
4 files changed, 124 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/30633/7
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30633
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
Gerrit-Change-Number: 30633
Gerrit-PatchSet: 7
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: msuraev.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30685 )
Change subject: osmo_stream_srv_link_close(): properly handle NULL input
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> I think library shouldn't suddenly crash it's user due to omitted NULL-check. […]
In my experiencie this ends up people not caring about what the hell is passed to functions and then ending up in a situation which nobody knows anymore what is expected in there, nor it's easy to follow lifecycle of objects. As said, if others are happy with this they are free to support merging this.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30685
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Ie15bb3dc99bfe18065e03fde68d517b0d389b7ad
Gerrit-Change-Number: 30685
Gerrit-PatchSet: 1
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 19 Dec 2022 17:00:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: comment
