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@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@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@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@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@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; -}
-
fixeria