Attention is currently required from: laforge, pespin.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169 )
Change subject: fix umts_cell_id_name(): show CID
......................................................................
Patch Set 3:
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169/comment/cd049b6e_17c7f84f
PS3, Line 12: Use OTC_SELECT
That's usually my opinion too, but since I have
the feeling I'm usually the only one against this I […]
My opinion is firmly
with dynamic allocation.
IIRC there is a log line that prints two distinct cell IDs. For that alone, i prefer the
dynamic approach.
But I'm really surprised to hear any praise for static buffers at all.
I thought that the consensus and aim is to get away from static buffers, to get away from
their notorious disadvantages:
- each static buffer can be used only once per va (LOGP(), printf()), or we get bugs and
misleading logs that are hard to spot and identify as such.
- there is a distinct static buffer for every API / API-function, so the vast vast
majority of static buffers just sit around 99.999% of the time simply taking up space and
not being used.
- truncation / a static buffer needs to be as large as the largest string that is at all
possible.
A long while back I proposed a kind of static ring buffer that alleviates the worst
disadvantages of static buffers, but the code review answer was the OTC_SELECT, which
after all I agree is a very elegant solution.
So now we go back from elegant to cumbersome???
If you mean to imply that this is inefficient and expensive related to the rest of
osmo-hnbgw, then kindly provide the performance measurements that prove it. <- the
standard answer for all premature optimization requests.
Patchset:
PS3:
I would gladly change my mind, but so far am not convinced...
File src/osmo-hnbgw/hnbgw.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169/comment/9d8261c8_54408599
PS2, Line 348: return talloc_asprintf(OTC_SELECT, "%u-%u-L%u-R%u-S%u-C%u",
ucid->mcc, ucid->mnc, ucid->lac, ucid->rac,
I hope this is not called everytime we log a line :)
i don't see a problem if it is.
We do these kinds of function calls as well as OTC_SELECT allocations A LOT for very many
log statements.
Noteworthy: producing log lines and calling this function is avoided entirely when the
logging category is silenced. So if a user has performance issues, they can crank up the
log levels.
If you mean to imply that this is inefficient and expensive related to the rest of
osmo-hnbgw, then kindly provide the performance measurements that prove it. <- the
standard answer for all premature optimization requests.
--
To view, visit
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Id903b8579ded8b908e644808e440d373bcca8da4
Gerrit-Change-Number: 33169
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Jun 2023 21:22:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment