ctrl: fix deferred commands (and hence fix osmo-bts-sysmo 'clock-info' cmd)

The CTRL interface has a ctrl_cmd_def_* API that allows deferring a CTRL
command reply until later. However, the command handling currently fails to
acknowledge this and deallocates the struct ctrl_cmd anyway.

Fix: in struct ctrl_cmd, add a defer pointer to be populated by
ctrl_cmd_def_make(). A cmd thus marked as deferred is not deallocated at the
end of command handling. This fix needs no change in calling code.

(Another idea was to return a different code than CTRL_CMD_HANDLED when the
command is to be deferred, but that would require adjusting each user of
ctrl_cmd_def_make(). The implicit marking is safer and easier.)

Show that handling deferred commands is fixed by adjusting the expectations of
ctrl_test.c's test_deferred_cmd() and removing the now obsolete exit_early
label.

One symptom of the breakage is that osmo-bts-sysmo crashes when asked to report
a trx's clock-info, which is aggravated by the fact that the sysmobts-mgr does
ask osmo-bts-sysmo for a clock-info.

The crash appears since Id583b413f8b8bd16e5cf92a8a9e8663903646381 -- it looked
like just fixing an obvious memory leak, which it did as shown by the unit
test, but deferred ctrl commands actually relied on that leak. Both fixed now.

Related: OS#3120
Change-Id: I24232be7dcf7be79f4def91ddc8b8f8005b56318

include/osmocom/ctrl/control_cmd.h

@@ -56,6 +56,8 @@ struct ctrl_connection {
 	struct llist_head def_cmds;
 };
 
+struct ctrl_cmd_def;
+
 struct ctrl_cmd {
 	struct ctrl_connection *ccon;
 	enum ctrl_type type;
@@ -64,6 +66,7 @@ struct ctrl_cmd {
 	char *variable;
 	char *value;
 	char *reply;
+	struct ctrl_cmd_def *defer;
 };
 
 #define ctrl_cmd_reply_printf(cmd, fmt, args ...) \

src/ctrl/control_cmd.c

@@ -566,6 +566,7 @@ ctrl_cmd_def_make(const void *ctx, struct ctrl_cmd *cmd, void *data, unsigned in
 
 	cd = talloc_zero(ctx, struct ctrl_cmd_def);
 
+	cmd->defer = cd;
 	cd->cmd = cmd;
 	cd->data = data;
 

src/ctrl/control_if.c

@@ -401,6 +401,13 @@ int ctrl_handle_msg(struct ctrl_handle *ctrl, struct ctrl_connection *ccon, stru
 	if (cmd->type != CTRL_TYPE_ERROR) {
 		cmd->ccon = ccon;
 		if (ctrl_cmd_handle(ctrl, cmd, ctrl->data) == CTRL_CMD_HANDLED) {
+
+			if (cmd->defer) {
+				/* The command is still stored as ctrl_cmd_def.cmd, in the def_cmds list.
+				 * Just leave hanging for deferred handling. Reply will happen later. */
+				return 0;
+			}
+
 			/* On CTRL_CMD_HANDLED, no reply needs to be sent back. */
 			talloc_free(cmd);
 			cmd = NULL;

tests/ctrl/ctrl_test.c

@@ -401,8 +401,6 @@ static void test_deferred_cmd()
 	/* Expecting a ctrl_cmd_def as well as the cmd to still be allocated */
 	if (talloc_total_size(ctx) <= ctx_size_before_defer) {
 		printf("deferred command apparently deallocated too soon\n");
-		/* ERROR -- showing current bug in handling deallocated cmds, hence exiting early */
-		goto exit_early;
 		talloc_report_full(ctx, stdout);
 		OSMO_ASSERT(false);
 	}
@@ -417,8 +415,6 @@ static void test_deferred_cmd()
 		OSMO_ASSERT(false);
 	}
 
-exit_early:
-
 	talloc_free(ccon);
 	talloc_free(ctrl);
 

tests/ctrl/ctrl_test.ok

@@ -176,5 +176,6 @@ ok
 test_deferred_cmd
 get_test_defer called
 ctrl_handle_msg() returned 0
-deferred command apparently deallocated too soon
+invoking ctrl_test_defer_cb() asynchronously
+ctrl_test_defer_cb called
 success