December 2022 - gerrit-log - lists.osmocom.org

Change in osmo-bsc[master]: WIP: vty: Add check against sensible default value for ny1

by arehbein

Attention is currently required from: pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/30602 ) Change subject: WIP: vty: Add check against sensible default value for ny1 ...................................................................... Patch Set 2: (2 comments) This change is ready for review. File src/osmo-bsc/bts.c: https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/8acd32f4_09665f3d PS1, Line 535: unsigned long ny1; > Lots of code here given that you already have a helper func.... […] I didn't want to move those lines into the function, because @bts.c@ doesn't include any logging, so I thought it isn't supposed to have any logging functions. Also on startup, a wrong value of Ny1 is supposed to lead to a connection shutdown, but during runtime, you mentioned logging a notice. Hence I thought different logs maybe. Anyways, then I'll assume it's okay to include the needed headers for logging in @bts.c@? File src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c: https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/4cb3bf29_441c72d2 PS1, Line 96: unsigned long ny1; > I think this is not needed here if you are already testing it during BTS setup in the other file. I thought reconfiguration of Ny1 during runtime might be a scenario which is why I added both checks (I think the config check only checks on startup...). Not important? -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/30602 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: If3f96a6bd4f9ae32b6421de43c1c5a5d64482089 Gerrit-Change-Number: 30602 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Dec 2022 00:11:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 6 months

1
0
0 0

Change in osmocom-bb[master]: mobile: rework writing BA to file, move to a function

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30614 ) Change subject: mobile: rework writing BA to file, move to a function ...................................................................... mobile: rework writing BA to file, move to a function Sometimes I am seeing error messages like this: DCS ERROR Failed to write BA list The problem is that there can be several BA entries which need to be written, and for each of them we're calling fwrite() twice. This function returns number of items written, so the final sum of returned values would be: len(BA list) * 2. Thus expecting it to be 2 regardless of len(BA list) is wrong. Fix this by checking the sum in each iteration, not at the end. Take a chance to refactor the code and move to a function. Change-Id: Id8bc216c146127d9c9995379c9e56450d328f46d --- M src/host/layer23/src/mobile/gsm322.c 1 file changed, 49 insertions(+), 31 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/14/30614/1 diff --git a/src/host/layer23/src/mobile/gsm322.c b/src/host/layer23/src/mobile/gsm322.c index d288596..9b3806f 100644 --- a/src/host/layer23/src/mobile/gsm322.c +++ b/src/host/layer23/src/mobile/gsm322.c @@ -5123,17 +5123,60 @@ return 0; } +static void gsm322_write_ba(struct osmocom_ms *ms) +{ + const struct gsm322_ba_list *ba; + char *ba_filename; + FILE *fp; + + ba_filename = talloc_asprintf(ms, "%s/%s.ba", config_dir, ms->name); + OSMO_ASSERT(ba_filename != NULL); + + LOGP(DCS, LOGL_INFO, "Writing stored BA list to '%s'\n", ba_filename); + + fp = fopen(ba_filename, "w"); + talloc_free(ba_filename); + if (fp == NULL) { + LOGP(DCS, LOGL_ERROR, + "Failed to open '%s' for writing: %s\n", + ba_filename, strerror(errno)); + return; + } + + fputs(ba_version, fp); + + llist_for_each_entry(ba, &ms->cellsel.ba_list, entry) { + size_t rc = 0; + uint8_t buf[] = { + ba->mcc >> 8, ba->mcc & 0xff, + ba->mnc >> 8, ba->mnc & 0xff, + }; + + LOGP(DCS, LOGL_INFO, + "Writing stored BA list entry (mcc=%s mnc=%s %s, %s)\n", + gsm_print_mcc(ba->mcc), gsm_print_mnc(ba->mnc), + gsm_get_mcc(ba->mcc), gsm_get_mnc(ba->mcc, ba->mnc)); + + rc += fwrite(buf, sizeof(buf), 1, fp); + rc += fwrite(ba->freq, sizeof(ba->freq), 1, fp); + + /* fwrite() returns count of written items, should be 2 */ + if (rc != 2) { + LOGP(DCS, LOGL_ERROR, + "Writing stored BA list: fwrite() failed (rc=%zu)\n", rc); + break; + } + } + + fclose(fp); +} + int gsm322_exit(struct osmocom_ms *ms) { struct gsm322_plmn *plmn = &ms->plmn; struct gsm322_cellsel *cs = &ms->cellsel; struct llist_head *lh, *lh2; struct msgb *msg; - FILE *fp; - char *ba_filename; - struct gsm322_ba_list *ba; - uint8_t buf[4]; - int rc = 0; int i; LOGP(DPLMN, LOGL_INFO, "exit PLMN process\n"); @@ -5161,32 +5204,7 @@ cs->si = NULL; /* store BA list */ - ba_filename = talloc_asprintf(ms, "%s/%s.ba", config_dir, ms->name); - if (ba_filename) { - fp = fopen(ba_filename, "w"); - talloc_free(ba_filename); - if (fp) { - fputs(ba_version, fp); - llist_for_each_entry(ba, &cs->ba_list, entry) { - buf[0] = ba->mcc >> 8; - buf[1] = ba->mcc & 0xff; - buf[2] = ba->mnc >> 8; - buf[3] = ba->mnc & 0xff; - - LOGP(DCS, LOGL_INFO, "Write stored BA list (mcc=%s " - "mnc=%s %s, %s)\n", gsm_print_mcc(ba->mcc), - gsm_print_mnc(ba->mnc), gsm_get_mcc(ba->mcc), - gsm_get_mnc(ba->mcc, ba->mnc)); - - rc += fwrite(buf, 4, 1, fp); - rc += fwrite(ba->freq, sizeof(ba->freq), 1, fp); - } - fclose(fp); - } - } - - if (rc != 2) - LOGP(DCS, LOGL_ERROR, "Failed to write BA list\n"); + gsm322_write_ba(ms); /* free lists */ while ((msg = msgb_dequeue(&plmn->event_queue))) -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30614 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Id8bc216c146127d9c9995379c9e56450d328f46d Gerrit-Change-Number: 30614 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 6 months

1
0
0 0

Change in osmocom-bb[master]: layer23: fix rx_l1_sim_conf(): msg->l2h is NULL, use msg->l1h

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30613 ) Change subject: layer23: fix rx_l1_sim_conf(): msg->l2h is NULL, use msg->l1h ...................................................................... layer23: fix rx_l1_sim_conf(): msg->l2h is NULL, use msg->l1h The mobile app crashes when using a Calypso phone and specifically when using its built-in SIM reader. The problem is that msg->l2h is NULL in rx_l1_sim_conf(), so msg->l1h must be used instead. Assert failed msgb->l2h /usr/local/include/osmocom/core/msgb.h:162 Change-Id: I7c68a3ad393be5fd0413e00e119a06db59672357 --- M src/host/layer23/src/common/l1ctl.c 1 file changed, 1 insertion(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/13/30613/1 diff --git a/src/host/layer23/src/common/l1ctl.c b/src/host/layer23/src/common/l1ctl.c index 7482788..94079c7 100644 --- a/src/host/layer23/src/common/l1ctl.c +++ b/src/host/layer23/src/common/l1ctl.c @@ -736,10 +736,7 @@ /* just forward the SIM response to the SIM handler */ static int rx_l1_sim_conf(struct osmocom_ms *ms, struct msgb *msg) { - uint16_t len = msgb_l2len(msg); - uint8_t *data = msg->data; - - LOGP(DL1C, LOGL_INFO, "SIM %s\n", osmo_hexdump(data, len)); + LOGP(DL1C, LOGL_INFO, "SIM %s\n", msgb_hexdump_l1(msg)); sim_apdu_resp(ms, msg); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30613 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I7c68a3ad393be5fd0413e00e119a06db59672357 Gerrit-Change-Number: 30613 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 6 months

1
0
0 0

Change in libosmo-gprs[master]: llc: Proper separation of public & private APIs, Introduce llc_prim API

by pespin

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/30489 ) Change subject: llc: Proper separation of public & private APIs, Introduce llc_prim API ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: I'm planning to merge this tomorrow or so in order to have it already in libosmo-gprs. More work is going to be needed anyway but a least it serves as a good base. -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/30489 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I588eb576b2703262f4ab9566ec362920d8390cfd Gerrit-Change-Number: 30489 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 14 Dec 2022 20:18:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 6 months

1
0
0 0

Change in osmocom-bb[master]: trxcon: move FBSB timeout calculation to the trxcon_fsm

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30611 ) Change subject: trxcon: move FBSB timeout calculation to the trxcon_fsm ...................................................................... trxcon: move FBSB timeout calculation to the trxcon_fsm In the upcoming patches I am adding a possibility to enlarge the FBSB timeout by providing API for that. This is needed for SDR based PHYs, because for them it takes longer to tune and so on. The L1CTL codec is not the right place for applying PHY specific quirks, so let's move the TDMA FNs -> ms conversion to the FSM logic. Change-Id: I685f48cfed000997b0d7c16073c6387bc05d2bbe Related: OS#5599 --- M src/host/trxcon/include/osmocom/bb/trxcon/trxcon_fsm.h M src/host/trxcon/src/l1ctl.c M src/host/trxcon/src/trxcon_fsm.c 3 files changed, 9 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/11/30611/1 diff --git a/src/host/trxcon/include/osmocom/bb/trxcon/trxcon_fsm.h b/src/host/trxcon/include/osmocom/bb/trxcon/trxcon_fsm.h index ec0b2c6..49b5e3a 100644 --- a/src/host/trxcon/include/osmocom/bb/trxcon/trxcon_fsm.h +++ b/src/host/trxcon/include/osmocom/bb/trxcon/trxcon_fsm.h @@ -54,7 +54,7 @@ /* param of TRXCON_EV_FBSB_SEARCH_REQ */ struct trxcon_param_fbsb_search_req { uint16_t band_arfcn; - uint16_t timeout_ms; + uint16_t timeout_fns; /* in TDMA Fn periods */ uint8_t pchan_config; }; diff --git a/src/host/trxcon/src/l1ctl.c b/src/host/trxcon/src/l1ctl.c index e424524..b0516cd 100644 --- a/src/host/trxcon/src/l1ctl.c +++ b/src/host/trxcon/src/l1ctl.c @@ -35,8 +35,8 @@ #include <osmocom/core/logging.h> #include <osmocom/core/utils.h> -#include <osmocom/gsm/gsm0502.h> #include <osmocom/gsm/gsm_utils.h> +#include <osmocom/gsm/protocol/gsm_04_08.h> #include <osmocom/gsm/protocol/gsm_08_58.h> #include <osmocom/bb/trxcon/logging.h> @@ -369,15 +369,15 @@ struct trxcon_param_fbsb_search_req req = { .pchan_config = l1ctl_ccch_mode2pchan_config(fbsb->ccch_mode), - .timeout_ms = ntohs(fbsb->timeout) * GSM_TDMA_FN_DURATION_uS / 1000, + .timeout_fns = ntohs(fbsb->timeout), .band_arfcn = ntohs(fbsb->band_arfcn), }; LOGPFSMSL(fi, g_logc_l1c, LOGL_NOTICE, - "Received FBSB request (%s %d, timeout %u ms)\n", + "Received FBSB request (%s %d, timeout %u TDMA FNs)\n", arfcn2band_name(req.band_arfcn), req.band_arfcn & ~ARFCN_FLAG_MASK, - req.timeout_ms); + req.timeout_fns); osmo_fsm_inst_dispatch(fi, TRXCON_EV_FBSB_SEARCH_REQ, &req); diff --git a/src/host/trxcon/src/trxcon_fsm.c b/src/host/trxcon/src/trxcon_fsm.c index 47da0bd..7c0057a 100644 --- a/src/host/trxcon/src/trxcon_fsm.c +++ b/src/host/trxcon/src/trxcon_fsm.c @@ -28,6 +28,7 @@ #include <osmocom/core/msgb.h> #include <osmocom/core/talloc.h> #include <osmocom/core/logging.h> +#include <osmocom/gsm/gsm0502.h> #include <osmocom/bb/trxcon/trxcon.h> #include <osmocom/bb/trxcon/trxcon_fsm.h> @@ -158,8 +159,10 @@ case TRXCON_EV_FBSB_SEARCH_REQ: { const struct trxcon_param_fbsb_search_req *req = data; + unsigned long timeout_ms; - osmo_fsm_inst_state_chg_ms(fi, TRXCON_ST_FBSB_SEARCH, req->timeout_ms, 0); + timeout_ms = req->timeout_fns * GSM_TDMA_FN_DURATION_uS / 1000; + osmo_fsm_inst_state_chg_ms(fi, TRXCON_ST_FBSB_SEARCH, timeout_ms, 0); l1sched_configure_ts(trxcon->sched, 0, req->pchan_config); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30611 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I685f48cfed000997b0d7c16073c6387bc05d2bbe Gerrit-Change-Number: 30611 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 6 months

1
0
0 0

Change in osmocom-bb[master]: trxcon: allow advancing FBSB timeout (quirks for slow PHYs)

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30612 ) Change subject: trxcon: allow advancing FBSB timeout (quirks for slow PHYs) ...................................................................... trxcon: allow advancing FBSB timeout (quirks for slow PHYs) This is needed for SDR based PHYs, because for them it takes longer to tune, flush the buffers and so on. Add a field to the trxcon_inst structure and a command line option (-F) for the trxcon app. Change-Id: Ia68954c5bdacda45fc871ffea0ccdf2460936408 Related: OS#5599 --- M src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h M src/host/trxcon/src/trxcon_fsm.c M src/host/trxcon/src/trxcon_inst.c M src/host/trxcon/src/trxcon_main.c 4 files changed, 27 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/12/30612/1 diff --git a/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h b/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h index 9148771..10527e3 100644 --- a/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h +++ b/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h @@ -32,6 +32,12 @@ uint8_t tx_power; int8_t ta; } l1p; + + /* PHY specific quirks */ + struct { + /* FBSB timeout advance (in TDMA FNs) */ + unsigned int fbsb_advance_fns; + } phy_quirks; }; enum trxcon_log_cat { diff --git a/src/host/trxcon/src/trxcon_fsm.c b/src/host/trxcon/src/trxcon_fsm.c index 7c0057a..ea7471b 100644 --- a/src/host/trxcon/src/trxcon_fsm.c +++ b/src/host/trxcon/src/trxcon_fsm.c @@ -159,9 +159,11 @@ case TRXCON_EV_FBSB_SEARCH_REQ: { const struct trxcon_param_fbsb_search_req *req = data; - unsigned long timeout_ms; + unsigned long timeout_fns, timeout_ms; - timeout_ms = req->timeout_fns * GSM_TDMA_FN_DURATION_uS / 1000; + /* Some PHYs need additional time to tune (in TDMA FNs) */ + timeout_fns = req->timeout_fns + trxcon->phy_quirks.fbsb_advance_fns; + timeout_ms = timeout_fns * GSM_TDMA_FN_DURATION_uS / 1000; osmo_fsm_inst_state_chg_ms(fi, TRXCON_ST_FBSB_SEARCH, timeout_ms, 0); l1sched_configure_ts(trxcon->sched, 0, req->pchan_config); diff --git a/src/host/trxcon/src/trxcon_inst.c b/src/host/trxcon/src/trxcon_inst.c index 7d3813e..e967cd8 100644 --- a/src/host/trxcon/src/trxcon_inst.c +++ b/src/host/trxcon/src/trxcon_inst.c @@ -91,6 +91,8 @@ return NULL; } + trxcon->phy_quirks.fbsb_advance_fns = 0; + return trxcon; } diff --git a/src/host/trxcon/src/trxcon_main.c b/src/host/trxcon/src/trxcon_main.c index 95e5549..0f9e616 100644 --- a/src/host/trxcon/src/trxcon_main.c +++ b/src/host/trxcon/src/trxcon_main.c @@ -67,6 +67,9 @@ uint16_t trx_base_port; uint32_t trx_fn_advance; + /* PHY quirk: FBSB timeout advance (in TDMA FNs) */ + int phyq_fbsb_advance_fns; + /* GSMTAP specific */ struct gsmtap_inst *gsmtap; const char *gsmtap_ip; @@ -77,6 +80,7 @@ .trx_bind_ip = "0.0.0.0", .trx_base_port = 6700, .trx_fn_advance = 3, + .phyq_fbsb_advance_fns = 0, }; static void *tall_trxcon_ctx = NULL; @@ -153,6 +157,7 @@ } trxcon->gsmtap = app_data.gsmtap; + trxcon->phy_quirks.fbsb_advance_fns = app_data.phyq_fbsb_advance_fns; } static void l1ctl_conn_close_cb(struct l1ctl_client *l1c) @@ -179,6 +184,7 @@ printf(" -i --trx-remote TRX remote IP address (default 127.0.0.1)\n"); printf(" -p --trx-port Base port of TRX instance (default 6700)\n"); printf(" -f --trx-advance Uplink burst scheduling advance (default 3)\n"); + printf(" -F --fbsb-advance FBSB timeout advance (in TDMA FNs, default 0)\n"); printf(" -s --socket Listening socket for layer23 (default /tmp/osmocom_l2)\n"); printf(" -g --gsmtap-ip The destination IP used for GSMTAP (disabled by default)\n"); printf(" -C --max-clients Maximum number of L1CTL connections (default 1)\n"); @@ -200,13 +206,14 @@ {"trx-remote", 1, 0, 'i'}, {"trx-port", 1, 0, 'p'}, {"trx-advance", 1, 0, 'f'}, + {"fbsb-advance", 1, 0, 'F'}, {"gsmtap-ip", 1, 0, 'g'}, {"max-clients", 1, 0, 'C'}, {"daemonize", 0, 0, 'D'}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, "d:b:i:p:f:s:g:C:Dh", + c = getopt_long(argc, argv, "d:b:i:p:f:F:s:g:C:Dh", long_options, &option_index); if (c == -1) break; @@ -232,6 +239,13 @@ case 'f': app_data.trx_fn_advance = atoi(optarg); break; + case 'F': + app_data.phyq_fbsb_advance_fns = atoi(optarg); + if (app_data.phyq_fbsb_advance_fns < 0) { + fprintf(stderr, "Error: FBSB advance shall be a positive value\n"); + exit(1); + } + break; case 's': app_data.bind_socket = optarg; break; -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30612 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ia68954c5bdacda45fc871ffea0ccdf2460936408 Gerrit-Change-Number: 30612 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 6 months

1
0
0 0

Change in libosmocore[master]: logging: rework the macros

by Jenkins Builder

Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30610 ) Change subject: logging: rework the macros ...................................................................... Patch Set 1: (1 comment) File src/logging.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-1936): https://gerrit.osmocom.org/c/libosmocore/+/30610/comment/5bb46356_8c22fa61 PS1, Line 779: if(!cont) space required before the open parenthesis '(' -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30610 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I03efa954cb9e991d2c3da4b61b12aac651e0efa2 Gerrit-Change-Number: 30610 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Wed, 14 Dec 2022 19:04:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 6 months

1
0
0 0

Change in libosmocore[master]: logging: do not expose internal data structures

by Jenkins Builder

Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30609 ) Change subject: logging: do not expose internal data structures ...................................................................... Patch Set 1: (1 comment) File src/logging.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-1935): https://gerrit.osmocom.org/c/libosmocore/+/30609/comment/30cfd46c_6f1ef89d PS1, Line 1424: int log_initialized(void) { open brace '{' following function definitions go on the next line -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30609 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I2ba3506233ff485f3935b2f46c94d8351b5d283c Gerrit-Change-Number: 30609 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Wed, 14 Dec 2022 19:03:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 6 months

1
0
0 0

Change in libosmocore[master]: logging: do not expose internal data structures

by Hoernchen

Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/30609 ) Change subject: logging: do not expose internal data structures ...................................................................... logging: do not expose internal data structures The header declares a log_initialized(void) but there never was a defintion (???!!) and instead of using the non existing function the struct was just exposed and abused (???!?!).... Change-Id: I2ba3506233ff485f3935b2f46c94d8351b5d283c --- M include/osmocom/core/logging.h M src/logging.c 2 files changed, 11 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/09/30609/1 diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h index 5ae38c7..f4201b1 100644 --- a/include/osmocom/core/logging.h +++ b/include/osmocom/core/logging.h @@ -11,8 +11,6 @@ #include <osmocom/core/defs.h> #include <osmocom/core/linuxlist.h> -extern struct log_info *osmo_log_info; - #ifndef DEBUG #define DEBUG #endif @@ -34,6 +32,7 @@ #define DEBUGPC(ss, fmt, args...) #endif +int log_initialized(void); void osmo_vlogp(int subsys, int level, const char *file, int line, int cont, const char *format, va_list ap); @@ -58,7 +57,7 @@ #ifndef LIBOSMOCORE_NO_LOGGING #define LOGPC(ss, level, fmt, args...) \ do { \ - if (!osmo_log_info) { \ + if (!log_initialized()) { \ logp_stub(__FILE__, __LINE__, 1, fmt, ##args); \ break; \ } \ @@ -100,7 +99,7 @@ #ifndef LIBOSMOCORE_NO_LOGGING #define LOGPSRCC(ss, level, caller_file, caller_line, cont, fmt, args...) \ do { \ - if (!osmo_log_info) { \ + if (!log_initialized()) { \ if (caller_file) \ logp_stub(caller_file, caller_line, cont, fmt, ##args); \ else \ @@ -399,7 +398,6 @@ __attribute__ ((format (printf, 6, 7))); void logp_stub(const char *file, int line, int cont, const char *format, ...); int log_init(const struct log_info *inf, void *talloc_ctx); -int log_initialized(void); void log_fini(void); int log_check_level(int subsys, unsigned int level); diff --git a/src/logging.c b/src/logging.c index ce42e4c..7e360b7 100644 --- a/src/logging.c +++ b/src/logging.c @@ -84,7 +84,7 @@ osmo_static_assert(_LOG_FLT_COUNT <= 8*sizeof(((struct log_target*)NULL)->filter_map), enum_logging_filters_fit_in_log_target_filter_map); -struct log_info *osmo_log_info; +static struct log_info *osmo_log_info; static struct log_context log_context; void *tall_log_ctx = NULL; @@ -1418,6 +1418,13 @@ return rc; } +/*! Check if the the Osmocom logging core is initialized + * \returns 1 in case this is true + */ +int log_initialized(void) { + return tall_log_ctx != NULL; +} + /*! Initialize the Osmocom logging core * \param[in] inf Information regarding logging categories, could be NULL * \param[in] ctx talloc context for logging allocations -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30609 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I2ba3506233ff485f3935b2f46c94d8351b5d283c Gerrit-Change-Number: 30609 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 6 months

1
0
0 0

Change in libosmocore[master]: logging: rework the macros

by Hoernchen

Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/30610 ) Change subject: logging: rework the macros ...................................................................... logging: rework the macros For some reason plenty of internal code ended up in the header macros even though it does not require any macro features, one single logging func should just do the right thing and log to stderr if logging is not available yet, or check the level and log properly. Change-Id: I03efa954cb9e991d2c3da4b61b12aac651e0efa2 --- M include/osmocom/core/logging.h M src/logging.c 2 files changed, 34 insertions(+), 37 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/10/30610/1 diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h index f4201b1..852bd97 100644 --- a/include/osmocom/core/logging.h +++ b/include/osmocom/core/logging.h @@ -32,8 +32,6 @@ #define DEBUGPC(ss, fmt, args...) #endif -int log_initialized(void); - void osmo_vlogp(int subsys, int level, const char *file, int line, int cont, const char *format, va_list ap); @@ -57,12 +55,7 @@ #ifndef LIBOSMOCORE_NO_LOGGING #define LOGPC(ss, level, fmt, args...) \ do { \ - if (!log_initialized()) { \ - logp_stub(__FILE__, __LINE__, 1, fmt, ##args); \ - break; \ - } \ - if (log_check_level(ss, level)) \ - logp2(ss, level, __FILE__, __LINE__, 1, fmt, ##args); \ + logp2_with_check(ss, level, __FILE__, __LINE__, 1, fmt, ##args); \ } while(0) #else #define LOGPC(ss, level, fmt, args...) @@ -99,19 +92,10 @@ #ifndef LIBOSMOCORE_NO_LOGGING #define LOGPSRCC(ss, level, caller_file, caller_line, cont, fmt, args...) \ do { \ - if (!log_initialized()) { \ - if (caller_file) \ - logp_stub(caller_file, caller_line, cont, fmt, ##args); \ - else \ - logp_stub(__FILE__, __LINE__, cont, fmt, ##args); \ - break; \ - } \ - if (log_check_level(ss, level)) {\ - if (caller_file) \ - logp2(ss, level, caller_file, caller_line, cont, fmt, ##args); \ - else \ - logp2(ss, level, __FILE__, __LINE__, cont, fmt, ##args); \ - }\ + if (caller_file) \ + logp2_with_check(ss, level, caller_file, caller_line, cont, fmt, ##args); \ + else \ + logp2_with_check(ss, level, __FILE__, __LINE__, cont, fmt, ##args); \ } while(0) #else #define LOGPSRCC(ss, level, caller_file, caller_line, cont, fmt, args...) @@ -396,6 +380,9 @@ void logp2(int subsys, unsigned int level, const char *file, int line, int cont, const char *format, ...) __attribute__ ((format (printf, 6, 7))); +void logp2_with_check(int subsys, unsigned int level, const char *file, + int line, int cont, const char *format, ...) + __attribute__ ((format (printf, 6, 7))); void logp_stub(const char *file, int line, int cont, const char *format, ...); int log_init(const struct log_info *inf, void *talloc_ctx); void log_fini(void); diff --git a/src/logging.c b/src/logging.c index 7e360b7..aba9176 100644 --- a/src/logging.c +++ b/src/logging.c @@ -84,7 +84,7 @@ osmo_static_assert(_LOG_FLT_COUNT <= 8*sizeof(((struct log_target*)NULL)->filter_map), enum_logging_filters_fit_in_log_target_filter_map); -static struct log_info *osmo_log_info; +struct log_info *osmo_log_info; static struct log_context log_context; void *tall_log_ctx = NULL; @@ -762,16 +762,33 @@ TRACE(LIBOSMOCORE_LOG_DONE()); } -/* This logging function is used as a fallback when the logging framework is - * not is not properly initialized. */ -void logp_stub(const char *file, int line, int cont, const char *format, ...) +/*! logging function used by LOGP() macro, checks log level, + * and uses fallback fprintf to stderr if logging system is not ready yet. + * \param[in] subsys Logging sub-system + * \param[in] level Log level + * \param[in] file name of source code file + * \param[in] cont continuation (1) or new line (0) + * \param[in] format format string + */ +void logp2_with_check(int subsys, unsigned int level, const char *file, int line, int cont, const char *format, ...) { va_list ap; - if (!cont) - fprintf(stderr, "%s:%d ", file, line); - va_start(ap, format); - vfprintf(stderr, format, ap); - va_end(ap); + + TRACE(LIBOSMOCORE_LOG_START()); + if (tall_log_ctx == NULL) { + if(!cont) + fprintf(stderr, "%s:%d ", file, line); + va_start(ap, format); + vfprintf(stderr, format, ap); + va_end(ap); + } + + if (log_check_level(subsys, level)) { + va_start(ap, format); + osmo_vlogp(subsys, level, file, line, cont, format, ap); + va_end(ap); + } + TRACE(LIBOSMOCORE_LOG_DONE()); } /*! Register a new log target with the logging core @@ -1418,13 +1435,6 @@ return rc; } -/*! Check if the the Osmocom logging core is initialized - * \returns 1 in case this is true - */ -int log_initialized(void) { - return tall_log_ctx != NULL; -} - /*! Initialize the Osmocom logging core * \param[in] inf Information regarding logging categories, could be NULL * \param[in] ctx talloc context for logging allocations -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30610 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I03efa954cb9e991d2c3da4b61b12aac651e0efa2 Gerrit-Change-Number: 30610 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 6 months

1
0
0 0

2025

2024

2023

2022

gerrit-log December 2022