From: Holger Hans Peter Freyther holger@moiji-mobile.com
Certain attributes are read-only. Add a macro to make it more easy to define those. --- openbsc/include/openbsc/control_cmd.h | 20 ++++++++++++++++++ openbsc/src/osmo-bsc/osmo_bsc_ctrl.c | 38 +++-------------------------------- 2 files changed, 23 insertions(+), 35 deletions(-)
diff --git a/openbsc/include/openbsc/control_cmd.h b/openbsc/include/openbsc/control_cmd.h index 8aede15..df5ae23 100644 --- a/openbsc/include/openbsc/control_cmd.h +++ b/openbsc/include/openbsc/control_cmd.h @@ -172,6 +172,26 @@ static struct ctrl_cmd_element cmd_##cmdname = { \ .verify = &verify_##cmdname, \ }
+#define CTRL_CMD_DEFINE_RO(cmdname, cmdstr) \ +static int get_##cmdname(struct ctrl_cmd *cmd, void *data); \ +static inline int set_##cmdname(struct ctrl_cmd *cmd, void *data) \ +{ \ + cmd->reply = "Read Only attribute"; \ + return CTRL_CMD_ERROR; \ +} \ +static inline int verify_##cmdname(struct ctrl_cmd *cmd, const char *value, void *data) \ +{ \ + cmd->reply = "Read Only attribute"; \ + return 1; \ +} \ +static struct ctrl_cmd_element cmd_##cmdname = { \ + .name = cmdstr, \ + .param = NULL, \ + .get = &get_##cmdname, \ + .set = &set_##cmdname, \ + .verify = &verify_##cmdname, \ +} + struct gsm_network;
#endif /* _CONTROL_CMD_H */ diff --git a/openbsc/src/osmo-bsc/osmo_bsc_ctrl.c b/openbsc/src/osmo-bsc/osmo_bsc_ctrl.c index e32218d..d3593de 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_ctrl.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_ctrl.c @@ -58,7 +58,7 @@ void osmo_bsc_send_trap(struct ctrl_cmd *cmd, struct bsc_msc_connection *msc_con talloc_free(trap); }
-CTRL_CMD_DEFINE(msc_connection_status, "msc_connection_status"); +CTRL_CMD_DEFINE_RO(msc_connection_status, "msc_connection_status"); static int msc_connection_status = 0;
static int get_msc_connection_status(struct ctrl_cmd *cmd, void *data) @@ -70,17 +70,6 @@ static int get_msc_connection_status(struct ctrl_cmd *cmd, void *data) return CTRL_CMD_REPLY; }
-static int set_msc_connection_status(struct ctrl_cmd *cmd, void *data) -{ - return CTRL_CMD_ERROR; -} - -static int verify_msc_connection_status(struct ctrl_cmd *cmd, const char *value, void *data) -{ - cmd->reply = "Read-only property"; - return 1; -} - static int msc_connection_status_trap_cb(unsigned int subsys, unsigned int signal, void *handler_data, void *signal_data) { struct ctrl_cmd *cmd; @@ -114,7 +103,7 @@ static int msc_connection_status_trap_cb(unsigned int subsys, unsigned int signa return 0; }
-CTRL_CMD_DEFINE(bts_connection_status, "bts_connection_status"); +CTRL_CMD_DEFINE_RO(bts_connection_status, "bts_connection_status"); static int bts_connection_status = 0;
static int get_bts_connection_status(struct ctrl_cmd *cmd, void *data) @@ -126,17 +115,6 @@ static int get_bts_connection_status(struct ctrl_cmd *cmd, void *data) return CTRL_CMD_REPLY; }
-static int set_bts_connection_status(struct ctrl_cmd *cmd, void *data) -{ - return CTRL_CMD_ERROR; -} - -static int verify_bts_connection_status(struct ctrl_cmd *cmd, const char *value, void *data) -{ - cmd->reply = "Read-only property"; - return 1; -} - static int bts_connection_status_trap_cb(unsigned int subsys, unsigned int signal, void *handler_data, void *signal_data) { struct ctrl_cmd *cmd; @@ -496,7 +474,7 @@ err: return 1; }
-CTRL_CMD_DEFINE(bts_rf_state, "rf_state"); +CTRL_CMD_DEFINE_RO(bts_rf_state, "rf_state"); static int get_bts_rf_state(struct ctrl_cmd *cmd, void *data) { const char *oper, *admin, *policy; @@ -520,16 +498,6 @@ static int get_bts_rf_state(struct ctrl_cmd *cmd, void *data) return CTRL_CMD_REPLY; }
-static int set_bts_rf_state(struct ctrl_cmd *cmd, void *data) -{ - cmd->reply = "set is unimplemented"; - return CTRL_CMD_ERROR; -} - -static int verify_bts_rf_state(struct ctrl_cmd *cmd, const char *value, void *data) -{ - return 0; -}
CTRL_CMD_DEFINE(net_rf_lock, "rf_locked"); static int get_net_rf_lock(struct ctrl_cmd *cmd, void *data)
That's much better that adding dummy functions manually.
On 15.05.2014 11:24, Holger Freyther wrote:
From: Holger Hans Peter Freyther holger@moiji-mobile.com
Certain attributes are read-only. Add a macro to make it more easy to define those.
--- a/openbsc/include/openbsc/control_cmd.h +++ b/openbsc/include/openbsc/control_cmd.h +#define CTRL_CMD_DEFINE_RO(cmdname, cmdstr) \ +static int get_##cmdname(struct ctrl_cmd *cmd, void *data); \ +static inline int set_##cmdname(struct ctrl_cmd *cmd, void *data) \
Why inline? We basically need the addresses of these functions.
+{ \
- cmd->reply = "Read Only attribute"; \
- return CTRL_CMD_ERROR; \
+} \ +static inline int verify_##cmdname(struct ctrl_cmd *cmd, const char *value, void *data) \ +{ \
- cmd->reply = "Read Only attribute"; \
- return 1; \
+} \ +static struct ctrl_cmd_element cmd_##cmdname = { \
- .name = cmdstr, \
- .param = NULL, \
- .get = &get_##cmdname, \
- .set = &set_##cmdname, \
- .verify = &verify_##cmdname, \
+}
It's quite some code duplication. What about something like the following (even if the resulting 'forward' declarations are somewhat senseless). This would even work with the ';' in the 'calling' code:
#define CTRL_CMD_DEFINE_RO(cmdname, cmdstr) \ static int set_##cmdname(struct ctrl_cmd *cmd, void *data) \ { \ cmd->reply = "Read Only attribute"; \ return CTRL_CMD_ERROR; \ } \ static int verify_##cmdname(struct ctrl_cmd *cmd, const char *value, void *data) \ { \ cmd->reply = "Read Only attribute"; \ return 1; \ } \ CTRL_CMD_DEFINE(cmdname, cmdstr)
Jacob
On Thu, May 15, 2014 at 02:58:28PM +0200, Jacob Erlbeck wrote:
Why inline? We basically need the addresses of these functions.
Like it is done for most methods defined inside a header file. But you are right, we do not need the inline here.
It's quite some code duplication. What about something like the following (even if the resulting 'forward' declarations are somewhat senseless). This would even work with the ';' in the 'calling' code:
#define CTRL_CMD_DEFINE_RO(cmdname, cmdstr) \
...
CTRL_CMD_DEFINE(cmdname, cmdstr)
Good idea, I have done that and cleaned it up a bit further.
-
Holger Freyther -
Holger Hans Peter Freyther
-
Jacob Erlbeck