12 Mar
2023
12 Mar
'23
7:30 p.m.
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"gt;\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"gt;\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"gt;\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"gt;\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"gt;\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