March 2023 - gerrit-log - lists.osmocom.org

[S] Change in osmocom-bb[master]: layer23: Fix cmdline args not applied
by fixeria 13 Mar '23

13 Mar '23

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31354 ) Change subject: layer23: Fix cmdline args not applied ...................................................................... Patch Set 2: Code-Review-2 (2 comments) Commit Message: https://gerrit.osmocom.org/c/osmocom-bb/+/31354/comment/729130cf_69ed8c0e PS2, Line 10: chicken-and-egg problem > The point of having an MS already existing when parsing cmdline opts is the same as for the VTY: it allows setting stuff on the object directly. Ideally an MS should be allocated whenever we find an 'ms' node in the VTY config file. This might not be the case in case of the mobile app, but this is what we normally do in other, more actively maintained osmo-* projects. > The main problem here is that there's a init() function, which I'd like to get called *before* any other call to the layer23app API, so that the app has time to initialize in runtime whatever it wishes. Well, there is nothing that prevents the app specific code to initialize whatever it wishes if we parse argv[] first and store the parsed values in some intermediate variables like we already do. This way we can immediately use the values parsed from the command line. > That also includes l23_app_info() which is used during cmd line parsing. If you move cmdline parsing before l23_app_init(), you call l23_app_info() before l23_app_init() which looks wrong to me. See my other comment. I really dislike the idea of `l23_app_init()` being a function and returning a pointer to static info. By requiring all l23 apps to define the app info as a globally accessible const symbol we can access it at any time, even during the argv[] parsing. > But you still need to allocate the MS in some apps at l23_app_init() time because the MS object needs to be available during VTY config parsing. As I said above, MS objects should ideally be allocated *during* the VTY config parsing, and definitely not before. We should not make assumptions on what the user may want from us before parsing what he/she actually wants. > In any case imho those command lines should be deprecated at some point, and VTY commands be used instead for all apps now that we support VTY in the layer23 common code. For simple apps like `ccch_scan` and `ccch_scan` we still want to accept params via the command line. Nobody would like writing a config file just to specify an ARFCN for those, right? For more complex apps like `mobile` and `modem` we indeed should accept *all* parameters via the VTY config file. Patchset: PS2: > I already asked several times that I'd like to get an answer regading my proposals in the earlier co […] Hey Pau, sorry for late feedback. I had a more detailed look at the current concept of layer23 apps, and was negatively surprised by the overkill we have in there. Specifically functions `l23_app_info()` and `cfg_supported()`, which basically return a pointer to the app info struct, which is pretty much static itself. I cleaned up / refactored the code: https://gerrit.osmocom.org/c/osmocom-bb/+/31862 and fixed the argv[] parsing by reordering function calls: https://gerrit.osmocom.org/c/osmocom-bb/+/31863 This patch in its current stare is not making the situation better, but rather complicating everything even further. I think it should be abandoned, sorry. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31354 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I285a4908401659b69dac513dd1a4b07facb88c51 Gerrit-Change-Number: 31354 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 00:05:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: layer23: fix parsing of command line options
by fixeria 13 Mar '23

13 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31863 ) Change subject: layer23: fix parsing of command line options ...................................................................... layer23: fix parsing of command line options After the recent refactoring, parsing of the command line options is broken for some arguments. Specifically, the value of '-a'/'--arfcn' is ignored and hard-coded ARFCN=871 is used instead. The problem is that l23_app_init(), which allocates an MS state and sets the initial ARFCN, is called *before* handle_options(). So the cfg_test_arfcn is used before it gets overwritten from the argv[]. The usual approach in osmo-* apps is to parse the command line arguments first, and only then execute code which depends on configurable parameters. Let's follow this approach too. Change-Id: I77ca11c14561fa3fcb9add60ccea5b0b847a20c4 --- M src/host/layer23/src/common/main.c M src/host/layer23/src/mobile/main.c 2 files changed, 29 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/63/31863/1 diff --git a/src/host/layer23/src/common/main.c b/src/host/layer23/src/common/main.c index 31e7154..e4a15ee 100644 --- a/src/host/layer23/src/common/main.c +++ b/src/host/layer23/src/common/main.c @@ -254,14 +254,14 @@ print_copyright(); + handle_options(argc, argv); + rc = l23_app_init(); if (rc < 0) { fprintf(stderr, "Failed during l23_app_init()\n"); exit(1); } - handle_options(argc, argv); - if (l23_app_info.opt_supported & L23_OPT_VTY) { if (_vty_init() < 0) exit(1); diff --git a/src/host/layer23/src/mobile/main.c b/src/host/layer23/src/mobile/main.c index 3260bff..d24a802 100644 --- a/src/host/layer23/src/mobile/main.c +++ b/src/host/layer23/src/mobile/main.c @@ -255,6 +255,12 @@ print_copyright(); + rc = handle_options(argc, argv); + if (rc) { /* Abort in case of parsing errors */ + fprintf(stderr, "Error in command line options. Exiting.\n"); + return 1; + } + srand(time(NULL)); INIT_LLIST_HEAD(&ms_list); @@ -272,12 +278,6 @@ exit(1); } - rc = handle_options(argc, argv); - if (rc) { /* Abort in case of parsing errors */ - fprintf(stderr, "Error in command line options. Exiting.\n"); - return 1; - } - if (custom_cfg_file) { /* Use full path provided by user */ config_file = talloc_strdup(l23_ctx, custom_cfg_file); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31863 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I77ca11c14561fa3fcb9add60ccea5b0b847a20c4 Gerrit-Change-Number: 31863 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[M] Change in osmocom-bb[master]: layer23: refactor the application API concept
by fixeria 13 Mar '23

13 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31862 ) Change subject: layer23: refactor the application API concept ...................................................................... layer23: refactor the application API concept With this set of changes we have a cleaner l23 app architecture: * struct vty_app_info: all l23 applications must define this struct; * struct vty_app_info: *cfg_supported() becomes a mask of L23_OPT_*; * struct vty_app_info: explicitly set L23_OPT_* in all l23 apps; * drop l23_app_info(), there can be only one vty_app_info per an app; It's no more needed to obtain the vty_app_info by calling a function and checking the returned value against NULL everywhere. This kind of information is rather static (not dynamically composed) and needs not to be encapsulated into functions. Change-Id: I89004cd5308927305f79b102f7b695709148df6d --- M src/host/layer23/include/osmocom/bb/common/l23_app.h M src/host/layer23/src/common/apn.c M src/host/layer23/src/common/main.c M src/host/layer23/src/common/vty.c M src/host/layer23/src/misc/app_bcch_scan.c M src/host/layer23/src/misc/app_cbch_sniff.c M src/host/layer23/src/misc/app_ccch_scan.c M src/host/layer23/src/misc/app_cell_log.c M src/host/layer23/src/misc/app_echo_test.c M src/host/layer23/src/mobile/app_mobile.c M src/host/layer23/src/mobile/main.c M src/host/layer23/src/modem/app_modem.c 12 files changed, 87 insertions(+), 147 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/62/31862/1 diff --git a/src/host/layer23/include/osmocom/bb/common/l23_app.h b/src/host/layer23/include/osmocom/bb/common/l23_app.h index 8ae4ead..fd8c877 100644 --- a/src/host/layer23/include/osmocom/bb/common/l23_app.h +++ b/src/host/layer23/include/osmocom/bb/common/l23_app.h @@ -60,10 +60,10 @@ struct l23_app_info { const char *copyright; const char *contribution; - struct vty_app_info *vty_info; /* L23_OPT_VTY */ + const struct vty_app_info *vty_info; /* L23_OPT_VTY */ char *getopt_string; - int (*cfg_supported)(); + uint32_t opt_supported; /* mask of L23_OPT_* */ int (*cfg_print_help)(); int (*cfg_getopt_opt)(struct option **options); int (*cfg_handle_opt)(int c,const char *optarg); @@ -71,6 +71,7 @@ osmo_tundev_data_ind_cb_t tun_data_ind_cb; }; -extern struct l23_app_info *l23_app_info(); +/* all l23 apps must define this structure */ +extern const struct l23_app_info l23_app_info; #endif /* _L23_APP_H */ diff --git a/src/host/layer23/src/common/apn.c b/src/host/layer23/src/common/apn.c index 169e30e..62aa0ff 100644 --- a/src/host/layer23/src/common/apn.c +++ b/src/host/layer23/src/common/apn.c @@ -61,7 +61,6 @@ int apn_start(struct osmobb_apn *apn) { - struct l23_app_info *app_info = l23_app_info(); int rc; if (apn->started) @@ -69,8 +68,8 @@ LOGPAPN(LOGL_INFO, apn, "Opening TUN device %s\n", apn->cfg.dev_name); /* Set TUN library callback. Must have been configured by the app: */ - OSMO_ASSERT(app_info && app_info->tun_data_ind_cb); - osmo_tundev_set_data_ind_cb(apn->tun, app_info->tun_data_ind_cb); + OSMO_ASSERT(l23_app_info.tun_data_ind_cb); + osmo_tundev_set_data_ind_cb(apn->tun, l23_app_info.tun_data_ind_cb); osmo_tundev_set_dev_name(apn->tun, apn->cfg.dev_name); osmo_tundev_set_netns_name(apn->tun, apn->cfg.dev_netns_name); diff --git a/src/host/layer23/src/common/main.c b/src/host/layer23/src/common/main.c index e82457f..31e7154 100644 --- a/src/host/layer23/src/common/main.c +++ b/src/host/layer23/src/common/main.c @@ -71,40 +71,33 @@ static void print_help(void) { - int options = 0xff; - struct l23_app_info *app = l23_app_info(); - - if (app && app->cfg_supported != 0) - options = app->cfg_supported(); - printf(" Some help...\n"); printf(" -h --help this text\n"); printf(" -s --socket /tmp/osmocom_l2. Path to the unix " "domain socket (l2)\n"); - if (options & L23_OPT_SAP) + if (l23_app_info.opt_supported & L23_OPT_SAP) printf(" -S --sap /tmp/osmocom_sap. Path to the " "unix domain socket (BTSAP)\n"); - if (options & L23_OPT_ARFCN) + if (l23_app_info.opt_supported & L23_OPT_ARFCN) printf(" -a --arfcn NR The ARFCN to be used for layer2.\n"); - if (options & L23_OPT_TAP) + if (l23_app_info.opt_supported & L23_OPT_TAP) printf(" -i --gsmtap-ip The destination IP used for GSMTAP.\n"); - if (options & L23_OPT_VTY) + if (l23_app_info.opt_supported & L23_OPT_VTY) printf(" -c --config-file The path to the VTY configuration file.\n"); - if (options & L23_OPT_DBG) + if (l23_app_info.opt_supported & L23_OPT_DBG) printf(" -d --debug Change debug flags.\n"); - if (app && app->cfg_print_help) - app->cfg_print_help(); + if (l23_app_info.cfg_print_help != NULL) + l23_app_info.cfg_print_help(); } static void build_config(char **opt, struct option **option) { - struct l23_app_info *app; struct option *app_opp = NULL; int app_len = 0, len; @@ -119,13 +112,12 @@ }; - app = l23_app_info(); *opt = talloc_asprintf(l23_ctx, "hs:S:a:i:c:d:%s", - app && app->getopt_string ? app->getopt_string : ""); + l23_app_info.getopt_string ? l23_app_info.getopt_string : ""); len = ARRAY_SIZE(long_options); - if (app && app->cfg_getopt_opt) - app_len = app->cfg_getopt_opt(&app_opp); + if (l23_app_info.cfg_getopt_opt != NULL) + app_len = l23_app_info.cfg_getopt_opt(&app_opp); *option = talloc_zero_array(l23_ctx, struct option, len + app_len + 1); memcpy(*option, long_options, sizeof(long_options)); @@ -133,7 +125,7 @@ memcpy(*option + len, app_opp, app_len * sizeof(struct option)); } -static void handle_options(int argc, char **argv, struct l23_app_info *app) +static void handle_options(int argc, char **argv) { struct option *long_options; char *opt; @@ -173,8 +165,8 @@ log_parse_category_mask(osmo_stderr_target, optarg); break; default: - if (app && app->cfg_handle_opt) - app->cfg_handle_opt(c, optarg); + if (l23_app_info.cfg_handle_opt != NULL) + l23_app_info.cfg_handle_opt(c, optarg); break; } } @@ -200,32 +192,29 @@ static void print_copyright(void) { - struct l23_app_info *app; - app = l23_app_info(); - printf("%s" "%s\n" "License GPLv2+: GNU GPL version 2 or later <http://gnu.org/licenses/gpl.html>\n" "This is free software: you are free to change and redistribute it.\n" "There is NO WARRANTY, to the extent permitted by law.\n\n", - app && app->copyright ? app->copyright : "", - app && app->contribution ? app->contribution : ""); + l23_app_info.copyright ? l23_app_info.copyright : "", + l23_app_info.contribution ? l23_app_info.contribution : ""); } -static int _vty_init(struct l23_app_info *app) +static int _vty_init(void) { - static struct vty_app_info l23_vty_info = { - .name = "OsmocomBB", - .version = PACKAGE_VERSION, - }; + struct vty_app_info info; int rc; - OSMO_ASSERT(app->vty_info); - app->vty_info->tall_ctx = l23_ctx; - vty_init(app->vty_info ? : &l23_vty_info); + OSMO_ASSERT(l23_app_info.vty_info != NULL); + info = *l23_app_info.vty_info; + info.tall_ctx = l23_ctx; + + vty_init(&info); logging_vty_add_cmds(); - if (app->vty_init) - app->vty_init(); + + if (l23_app_info.vty_init != NULL) + l23_app_info.vty_init(); if (config_file) { LOGP(DLGLOBAL, LOGL_INFO, "Using configuration from '%s'\n", config_file); l23_vty_reading = true; @@ -249,8 +238,6 @@ int main(int argc, char **argv) { int rc; - struct l23_app_info *app; - unsigned int app_supp_opt = 0x00; INIT_LLIST_HEAD(&ms_list); @@ -273,18 +260,14 @@ exit(1); } - app = l23_app_info(); - if (app && app->cfg_supported != NULL) - app_supp_opt = app->cfg_supported(); + handle_options(argc, argv); - handle_options(argc, argv, app); - - if (app_supp_opt & L23_OPT_VTY) { - if (_vty_init(app) < 0) + if (l23_app_info.opt_supported & L23_OPT_VTY) { + if (_vty_init() < 0) exit(1); } - if (app_supp_opt & L23_OPT_TAP) { + if (l23_app_info.opt_supported & L23_OPT_TAP) { if (gsmtap_ip) { if (l23_cfg.gsmtap.remote_host != NULL) { LOGP(DLGLOBAL, LOGL_NOTICE, diff --git a/src/host/layer23/src/common/vty.c b/src/host/layer23/src/common/vty.c index 5987667..fd63918 100644 --- a/src/host/layer23/src/common/vty.c +++ b/src/host/layer23/src/common/vty.c @@ -559,18 +559,12 @@ int l23_vty_init(int (*config_write_ms_node_cb)(struct vty *), osmo_signal_cbfn *l23_vty_signal_cb) { - struct l23_app_info *app; - unsigned int app_supp_opt = 0x00; int rc = 0; - app = l23_app_info(); - if (app && app->cfg_supported != NULL) - app_supp_opt = app->cfg_supported(); - - if (app_supp_opt & L23_OPT_TAP) + if (l23_app_info.opt_supported & L23_OPT_TAP) l23_vty_init_gsmtap(); - if (app_supp_opt & L23_OPT_VTY) + if (l23_app_info.opt_supported & L23_OPT_VTY) osmo_stats_vty_add_cmds(); install_node(&ms_node, config_write_ms_node_cb); diff --git a/src/host/layer23/src/misc/app_bcch_scan.c b/src/host/layer23/src/misc/app_bcch_scan.c index 274c4df..768920d 100644 --- a/src/host/layer23/src/misc/app_bcch_scan.c +++ b/src/host/layer23/src/misc/app_bcch_scan.c @@ -75,12 +75,8 @@ return osmo_signal_register_handler(SS_L1CTL, &signal_cb, NULL); } -static struct l23_app_info info = { +const struct l23_app_info l23_app_info = { .copyright = "Copyright (C) 2010 Harald Welte <laforge(a)gnumonks.org>\n", .contribution = "Contributions by Holger Hans Peter Freyther\n", + .opt_supported = L23_OPT_ARFCN | L23_OPT_TAP | L23_OPT_DBG, }; - -struct l23_app_info *l23_app_info(void) -{ - return &info; -} diff --git a/src/host/layer23/src/misc/app_cbch_sniff.c b/src/host/layer23/src/misc/app_cbch_sniff.c index 64850db..dbf6d00 100644 --- a/src/host/layer23/src/misc/app_cbch_sniff.c +++ b/src/host/layer23/src/misc/app_cbch_sniff.c @@ -227,12 +227,8 @@ return osmo_signal_register_handler(SS_L1CTL, &signal_cb, NULL); } -static struct l23_app_info info = { +const struct l23_app_info l23_app_info = { .copyright = "Copyright (C) 2010 Harald Welte <laforge(a)gnumonks.org>\n", .contribution = "Contributions by Holger Hans Peter Freyther\n", + .opt_supported = L23_OPT_ARFCN | L23_OPT_TAP | L23_OPT_DBG, }; - -struct l23_app_info *l23_app_info(void) -{ - return &info; -} diff --git a/src/host/layer23/src/misc/app_ccch_scan.c b/src/host/layer23/src/misc/app_ccch_scan.c index be35b5b..7cbe267 100644 --- a/src/host/layer23/src/misc/app_ccch_scan.c +++ b/src/host/layer23/src/misc/app_ccch_scan.c @@ -519,12 +519,8 @@ return layer3_init(app_state.ms); } -static struct l23_app_info info = { +const struct l23_app_info l23_app_info = { .copyright = "Copyright (C) 2010 Harald Welte <laforge(a)gnumonks.org>\n", .contribution = "Contributions by Holger Hans Peter Freyther\n", + .opt_supported = L23_OPT_ARFCN | L23_OPT_TAP | L23_OPT_DBG, }; - -struct l23_app_info *l23_app_info(void) -{ - return &info; -} diff --git a/src/host/layer23/src/misc/app_cell_log.c b/src/host/layer23/src/misc/app_cell_log.c index 8c925af..9c49800 100644 --- a/src/host/layer23/src/misc/app_cell_log.c +++ b/src/host/layer23/src/misc/app_cell_log.c @@ -101,11 +101,6 @@ return 0; } -static int l23_cfg_supported(void) -{ - return L23_OPT_TAP | L23_OPT_DBG; -} - static int l23_getopt_options(struct option **options) { static struct option opts [] = { @@ -247,16 +242,11 @@ exit(1); } -static struct l23_app_info info = { +const struct l23_app_info l23_app_info = { .copyright = "Copyright (C) 2010 Andreas Eversberg\n", .getopt_string = "g:p:l:r:nf:b:A:", - .cfg_supported = l23_cfg_supported, + .opt_supported = L23_OPT_TAP | L23_OPT_DBG, .cfg_getopt_opt = l23_getopt_options, .cfg_handle_opt = l23_cfg_handle, .cfg_print_help = l23_cfg_print_help, }; - -struct l23_app_info *l23_app_info(void) -{ - return &info; -} diff --git a/src/host/layer23/src/misc/app_echo_test.c b/src/host/layer23/src/misc/app_echo_test.c index f8899e6..f5b3136 100644 --- a/src/host/layer23/src/misc/app_echo_test.c +++ b/src/host/layer23/src/misc/app_echo_test.c @@ -54,12 +54,7 @@ return 0; } -static struct l23_app_info info = { +const struct l23_app_info l23_app_info = { .copyright = "Copyright (C) 2010 Harald Welte <laforge(a)gnumonks.org>\n", .contribution = "Contributions by Holger Hans Peter Freyther\n", }; - -struct l23_app_info *l23_app_info(void) -{ - return &info; -} diff --git a/src/host/layer23/src/mobile/app_mobile.c b/src/host/layer23/src/mobile/app_mobile.c index efd3a75..a3b36e3 100644 --- a/src/host/layer23/src/mobile/app_mobile.c +++ b/src/host/layer23/src/mobile/app_mobile.c @@ -490,30 +490,19 @@ return ms_vty_init(); } -static int l23_cfg_supported(void) -{ - return L23_OPT_TAP | L23_OPT_VTY | L23_OPT_DBG; -} - -static struct vty_app_info _mobile_vty_info = { +static const struct vty_app_info _mobile_vty_info = { .name = "mobile", .version = PACKAGE_VERSION, }; -static struct l23_app_info info = { +const struct l23_app_info l23_app_info = { .copyright = "Copyright (C) 2010-2015 Andreas Eversberg, Sylvain Munaut, Holger Freyther, Harald Welte\n", .contribution = "Contributions by Alex Badea, Pablo Neira, Steve Markgraf and others\n", - .cfg_supported = &l23_cfg_supported, + .opt_supported = L23_OPT_TAP | L23_OPT_VTY | L23_OPT_DBG, .vty_info = &_mobile_vty_info, .vty_init = _mobile_vty_init, }; -struct l23_app_info *l23_app_info(void) -{ - return &info; -} - - void mobile_set_started(struct osmocom_ms *ms, bool state) { ms->started = state; diff --git a/src/host/layer23/src/mobile/main.c b/src/host/layer23/src/mobile/main.c index 43909ce..3260bff 100644 --- a/src/host/layer23/src/mobile/main.c +++ b/src/host/layer23/src/mobile/main.c @@ -206,32 +206,29 @@ static void print_copyright(void) { - struct l23_app_info *app; - app = l23_app_info(); - printf("%s" "%s\n" "License GPLv2+: GNU GPL version 2 or later <http://gnu.org/licenses/gpl.html>\n" "This is free software: you are free to change and redistribute it.\n" "There is NO WARRANTY, to the extent permitted by law.\n\n", - app && app->copyright ? app->copyright : "", - app && app->contribution ? app->contribution : ""); + l23_app_info.copyright ? l23_app_info.copyright : "", + l23_app_info.contribution ? l23_app_info.contribution : ""); } -static int _vty_init(struct l23_app_info *app) +static int _vty_init(void) { - static struct vty_app_info l23_vty_info = { - .name = "OsmocomBB", - .version = PACKAGE_VERSION, - }; + struct vty_app_info info; int rc; - OSMO_ASSERT(app->vty_info); - app->vty_info->tall_ctx = l23_ctx; - vty_init(app->vty_info ? : &l23_vty_info); + OSMO_ASSERT(l23_app_info.vty_info != NULL); + info = *l23_app_info.vty_info; + info.tall_ctx = l23_ctx; + + vty_init(&info); logging_vty_add_cmds(); - if (app->vty_init) - app->vty_init(); + + if (l23_app_info.vty_init != NULL) + l23_app_info.vty_init(); if (config_file) { LOGP(DLGLOBAL, LOGL_INFO, "Using configuration from '%s'\n", config_file); l23_vty_reading = true; @@ -255,8 +252,6 @@ int main(int argc, char **argv) { int rc; - struct l23_app_info *app; - unsigned int app_supp_opt = 0x00; print_copyright(); @@ -277,11 +272,6 @@ exit(1); } - app = l23_app_info(); - if (app && app->cfg_supported != NULL) - app_supp_opt = app->cfg_supported(); - - rc = handle_options(argc, argv); if (rc) { /* Abort in case of parsing errors */ fprintf(stderr, "Error in command line options. Exiting.\n"); @@ -306,8 +296,8 @@ config_dir = talloc_strdup(l23_ctx, config_file); config_dir = dirname(config_dir); - if (app_supp_opt & L23_OPT_VTY) { - if (_vty_init(app) < 0) + if (l23_app_info.opt_supported & L23_OPT_VTY) { + if (_vty_init() < 0) exit(1); } diff --git a/src/host/layer23/src/modem/app_modem.c b/src/host/layer23/src/modem/app_modem.c index b55768f..955a819 100644 --- a/src/host/layer23/src/modem/app_modem.c +++ b/src/host/layer23/src/modem/app_modem.c @@ -572,26 +572,16 @@ return 0; } -static int l23_cfg_supported(void) -{ - return L23_OPT_ARFCN | L23_OPT_TAP | L23_OPT_VTY | L23_OPT_DBG; -} - -static struct vty_app_info _modem_vty_info = { +static const struct vty_app_info _modem_vty_info = { .name = "modem", .version = PACKAGE_VERSION, .go_parent_cb = modem_vty_go_parent, }; -static struct l23_app_info info = { +const struct l23_app_info l23_app_info = { .copyright = "Copyright (C) 2022 by sysmocom - s.m.f.c. GmbH <info(a)sysmocom.de>\n", - .cfg_supported = &l23_cfg_supported, + .opt_supported = L23_OPT_ARFCN | L23_OPT_TAP | L23_OPT_VTY | L23_OPT_DBG, .vty_info = &_modem_vty_info, .vty_init = modem_vty_init, .tun_data_ind_cb = modem_tun_data_ind_cb, }; - -struct l23_app_info *l23_app_info(void) -{ - return &info; -} -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31862 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I89004cd5308927305f79b102f7b695709148df6d Gerrit-Change-Number: 31862 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in libosmo-netif[master]: src/Makefile.am: do not overwrite AM_LDFLAGS
by fixeria 12 Mar '23

12 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/31861 ) Change subject: src/Makefile.am: do not overwrite AM_LDFLAGS ...................................................................... src/Makefile.am: do not overwrite AM_LDFLAGS Change-Id: I7b656c167cceb0657c0362b1237f846b7f3e9770 --- M src/Makefile.am 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/61/31861/1 diff --git a/src/Makefile.am b/src/Makefile.am index 1e89e39..f12085d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,7 +9,7 @@ lib_LTLIBRARIES = libosmonetif.la libosmonetif_la_LIBADD = $(LIBOSMOCORE_LIBS) $(LIBOSMOGSM_LIBS) $(LIBSCTP_LIBS) -libosmonetif_la_LDFLAGS = -version-info $(LIBVERSION) -no-undefined +libosmonetif_la_LDFLAGS = $(AM_LDFLAGS) -version-info $(LIBVERSION) -no-undefined libosmonetif_la_SOURCES = amr.c \ datagram.c \ -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/31861 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I7b656c167cceb0657c0362b1237f846b7f3e9770 Gerrit-Change-Number: 31861 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in libosmo-netif[master]: {utils,tests}/Makefile.am: reorder libraries in LDADD
by fixeria 12 Mar '23

12 Mar '23

Attention is currently required from: fixeria. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/31775 to look at the new patch set (#2). Change subject: {utils,tests}/Makefile.am: reorder libraries in LDADD ...................................................................... {utils,tests}/Makefile.am: reorder libraries in LDADD Otherwise the linker may pick system-installed libs instead. Change-Id: Ia639b1c5460ad9391d2c311b4978ca9374789f7a --- M tests/Makefile.am M utils/Makefile.am 2 files changed, 19 insertions(+), 17 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/75/31775/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/31775 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ia639b1c5460ad9391d2c311b4978ca9374789f7a Gerrit-Change-Number: 31775 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in libasn1c[master]: src/Makefile.am: libraries shall not be in LDFLAGS but in LIBADD
by fixeria 12 Mar '23

12 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libasn1c/+/31860 ) Change subject: src/Makefile.am: libraries shall not be in LDFLAGS but in LIBADD ...................................................................... src/Makefile.am: libraries shall not be in LDFLAGS but in LIBADD Change-Id: I53dd51ab1f4666c42851a6cbdf1ee970530edee1 --- M src/Makefile.am 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libasn1c refs/changes/60/31860/1 diff --git a/src/Makefile.am b/src/Makefile.am index 29b5e64..019ff98 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -11,5 +11,14 @@ lib_LTLIBRARIES = libasn1c.la -libasn1c_la_LDFLAGS = $(LIBTALLOC_LIBS) -lm -version-info $(LIBVERSION) -no-undefined +libasn1c_la_LIBADD = \ + $(LIBTALLOC_LIBS) \ + -lm \ + $(NULL) + +libasn1c_la_LDFLAGS = \ + -version-info $(LIBVERSION) \ + -no-undefined \ + $(NULL) + libasn1c_la_SOURCES = ANY.c constraints.c GeneralizedTime.c NumericString.c T61String.c asn_codecs_prim.c constr_CHOICE.c GeneralString.c ObjectDescriptor.c TeletexString.c asn_SEQUENCE_OF.c constr_SEQUENCE.c GraphicString.c OBJECT_IDENTIFIER.c UniversalString.c asn_SET_OF.c constr_SEQUENCE_OF.c IA5String.c OCTET_STRING.c UTCTime.c ber_decoder.c constr_SET.c INTEGER.c per_decoder.c UTF8String.c ber_tlv_length.c constr_SET_OF.c ISO646String.c per_encoder.c VideotexString.c ber_tlv_tag.c constr_TYPE.c NativeEnumerated.c per_support.c VisibleString.c BIT_STRING.c NativeInteger.c PrintableString.c xer_decoder.c BMPString.c der_encoder.c NativeReal.c REAL.c xer_encoder.c BOOLEAN.c ENUMERATED.c NULL.c RELATIVE-OID.c xer_support.c per_opentype.c asn1helpers.c -- To view, visit https://gerrit.osmocom.org/c/libasn1c/+/31860 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libasn1c Gerrit-Branch: master Gerrit-Change-Id: I53dd51ab1f4666c42851a6cbdf1ee970530edee1 Gerrit-Change-Number: 31860 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in gapk[master]: src/Makefile.am: there shall be no libraries in LDFLAGS
by fixeria 12 Mar '23

12 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/gapk/+/31858 ) Change subject: src/Makefile.am: there shall be no libraries in LDFLAGS ...................................................................... src/Makefile.am: there shall be no libraries in LDFLAGS AM_LDFLAGS is for linker flags like '-no-undefined', not libraries! Change-Id: I7620e0c4e6444081e351e1dd021e0a8e2ae20337 --- M src/Makefile.am 1 file changed, 16 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/gapk refs/changes/58/31858/1 diff --git a/src/Makefile.am b/src/Makefile.am index 8fdeff7..9cbe492 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,12 +20,15 @@ lib_LTLIBRARIES = libosmogapk.la EXTRA_DIST = libosmogapk.map -libosmogapk_la_LDFLAGS = \ +libosmogapk_la_LIBADD = \ $(LIBOSMOCORE_LIBS) \ $(LIBOSMOCODEC_LIBS) \ ${OPENCORE_AMRNB_LIBS} \ ${LIBGSM_LIBS} \ $(LIBALSA_LIBS) \ + $(NULL) + +libosmogapk_la_LDFLAGS = \ $(LTLDFLAGS_OSMOGAPK) \ -version-info $(LIBVERSION) \ -no-undefined \ @@ -94,10 +97,7 @@ app_osmo_gapk.c \ $(NULL) -osmo_gapk_LDFLAGS = \ - $(LIBOSMOCORE_LIBS) \ - $(NULL) - osmo_gapk_LDADD = \ libosmogapk.la \ + $(LIBOSMOCORE_LIBS) \ $(NULL) -- To view, visit https://gerrit.osmocom.org/c/gapk/+/31858 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: gapk Gerrit-Branch: master Gerrit-Change-Id: I7620e0c4e6444081e351e1dd021e0a8e2ae20337 Gerrit-Change-Number: 31858 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in gapk[master]: Makefile.am: remove unused AM_CPPFLAGS variable
by fixeria 12 Mar '23

12 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/gapk/+/31857 ) Change subject: Makefile.am: remove unused AM_CPPFLAGS variable ...................................................................... Makefile.am: remove unused AM_CPPFLAGS variable Change-Id: I435e1153db2092c8f1632a84c1f546b98031e101 --- M Makefile.am 1 file changed, 9 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/gapk refs/changes/57/31857/1 diff --git a/Makefile.am b/Makefile.am index 0093f0e..a1642d4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4,8 +4,6 @@ pkgconfigdir = $(libdir)/pkgconfig pkgconfig_DATA = libosmogapk.pc -AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include - SUBDIRS = include if ENABLE_GSMHR -- To view, visit https://gerrit.osmocom.org/c/gapk/+/31857 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: gapk Gerrit-Branch: master Gerrit-Change-Id: I435e1153db2092c8f1632a84c1f546b98031e101 Gerrit-Change-Number: 31857 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in gapk[master]: libgsmhr: make use of unused LIBVERSION variable
by fixeria 12 Mar '23

12 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/gapk/+/31859 ) Change subject: libgsmhr: make use of unused LIBVERSION variable ...................................................................... libgsmhr: make use of unused LIBVERSION variable Change-Id: I4ef723bdd255fb81393806fdc500dacde2dd917f --- M libgsmhr/Makefile.am 1 file changed, 15 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/gapk refs/changes/59/31859/1 diff --git a/libgsmhr/Makefile.am b/libgsmhr/Makefile.am index f36d759..9904098 100644 --- a/libgsmhr/Makefile.am +++ b/libgsmhr/Makefile.am @@ -1,3 +1,8 @@ +# This is _NOT_ the library release version, it's an API version. +# Please read Chapter 6 "Library interface versions" of the libtool +# documentation before making any modification +LIBVERSION = 0:0:0 + AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include -I$(top_builddir) AM_CFLAGS = -fPIC -Wall ${SYMBOL_VISIBILITY} @@ -6,8 +11,6 @@ patches \ $(NULL) -LIBVERSION=0:0:0 - REFSRC_PATH=refsrc REFSRC_SRC=refsrc/dtx.c refsrc/globdefs.c refsrc/host.c refsrc/sp_enc.c refsrc/sp_rom.c refsrc/vad.c refsrc/err_conc.c refsrc/homing.c refsrc/mathdp31.c refsrc/sp_dec.c refsrc/sp_frm.c refsrc/sp_sfrm.c @@ -22,6 +25,7 @@ lib_LTLIBRARIES = libgsmhr.la libgsmhr_la_SOURCES = $(REFSRC_SRC) libgsmhr.c +libgsmhr_la_LDFLAGS = -version-info $(LIBVERSION) libgsmhr_la_LIBADD = -ldl clean-local: -- To view, visit https://gerrit.osmocom.org/c/gapk/+/31859 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: gapk Gerrit-Branch: master Gerrit-Change-Id: I4ef723bdd255fb81393806fdc500dacde2dd917f Gerrit-Change-Number: 31859 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-trx[master]: CommonLibs: remove unused *trx in cfg[_no]_ctr_error_threshold_cmd
by fixeria 12 Mar '23

12 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-trx/+/31856 ) Change subject: CommonLibs: remove unused *trx in cfg[_no]_ctr_error_threshold_cmd ...................................................................... CommonLibs: remove unused *trx in cfg[_no]_ctr_error_threshold_cmd Compilation fails when building with -Wall and -Werror: trx_vty.c: In function ‘cfg_ctr_error_threshold’: trx_vty.c:469:18: error: unused variable ‘trx’ [-Werror=unused-variable] struct trx_ctx *trx = trx_from_vty(vty); ^~~ trx_vty.c: In function ‘cfg_no_ctr_error_threshold’: trx_vty.c:501:18: error: unused variable ‘trx’ [-Werror=unused-variable] struct trx_ctx *trx = trx_from_vty(vty); ^~~ Change-Id: I3d74c3f38aa03608a32b9dee472e49236543c87f --- M CommonLibs/trx_vty.c 1 file changed, 20 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/56/31856/1 diff --git a/CommonLibs/trx_vty.c b/CommonLibs/trx_vty.c index 2b0ba71..ba55f92 100644 --- a/CommonLibs/trx_vty.c +++ b/CommonLibs/trx_vty.c @@ -466,7 +466,6 @@ int rc; struct ctr_threshold ctr; - struct trx_ctx *trx = trx_from_vty(vty); rc = vty_ctr_name_2_id(argv[0]); if (rc < 0) { vty_out(vty, "No valid ctr_name found for ctr-error-threshold %s%s", @@ -498,7 +497,6 @@ int rc; struct ctr_threshold ctr; - struct trx_ctx *trx = trx_from_vty(vty); rc = vty_ctr_name_2_id(argv[0]); if (rc < 0) { vty_out(vty, "No valid ctr_name found for ctr-error-threshold %s%s", -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/31856 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I3d74c3f38aa03608a32b9dee472e49236543c87f Gerrit-Change-Number: 31856 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0