obmc-console: Consolidate handling of default socket-id
If console-id is not specified on command line or in the config file
then use the default value. ae2460d0b8e8 ("obmc-console: Provide a
default value for `console-id`.") only implemented the default value for
naming the abstract listening socket and overlooked the new DBus path
naming convention. This caused issues during dbus registration:
```
obmc-console-server: Object name: /xyz/openbmc_project/console/(null)
obmc-console-server: Failed to issue method call: Invalid argument
```
Fixes: ae2460d0b8e8 ("obmc-console: Provide a default value for `console-id`.")
Change-Id: I6d0f7b23cc085992189cd4463129a6aae590b3e7
Signed-off-by: Ninad Palsule <ninadpalsule@us.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b891bad..18d095b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -39,3 +39,7 @@
Deprecate the `socket-id` key in the configuration schema. Uses of
`socket-id` should be directly replaced with `console-id`.
+
+### Fixed
+
+1. obmc-console: Consolidate handling of default socket-id
diff --git a/config.c b/config.c
index 707c024..3d7bd20 100644
--- a/config.c
+++ b/config.c
@@ -332,6 +332,29 @@
return 0;
}
+/* Default console id if not specified on command line or in config */
+#define DEFAULT_CONSOLE_ID "default"
+
+/* Get the console id */
+const char *config_resolve_console_id(struct config *config, const char *id_arg)
+{
+ const char *configured;
+
+ if (id_arg) {
+ return id_arg;
+ }
+
+ if ((configured = config_get_value(config, "console-id"))) {
+ return configured;
+ }
+
+ if ((configured = config_get_value(config, "socket-id"))) {
+ return configured;
+ }
+
+ return DEFAULT_CONSOLE_ID;
+}
+
#ifdef CONFIG_TEST
int main(void)
{
diff --git a/console-client.c b/console-client.c
index de2b1e3..3dc3190 100644
--- a/console-client.c
+++ b/console-client.c
@@ -212,8 +212,10 @@
return 0;
}
-static int client_init(struct console_client *client, const char *console_id)
+static int client_init(struct console_client *client, struct config *config,
+ const char *console_id)
{
+ const char *resolved_id = NULL;
struct sockaddr_un addr;
socket_path_t path;
ssize_t len;
@@ -225,9 +227,12 @@
return -1;
}
+ /* Get the console id */
+ resolved_id = config_resolve_console_id(config, console_id);
+
memset(&addr, 0, sizeof(addr));
addr.sun_family = AF_UNIX;
- len = console_socket_path(addr.sun_path, console_id);
+ len = console_socket_path(addr.sun_path, resolved_id);
if (len < 0) {
if (errno) {
warn("Failed to configure socket: %s", strerror(errno));
@@ -325,15 +330,6 @@
esc = (const uint8_t *)config_get_value(
config, "escape-sequence");
}
-
- if (!console_id) {
- console_id = config_get_value(config, "console-id");
- }
-
- /* socket-id is deprecated */
- if (!console_id) {
- console_id = config_get_value(config, "socket-id");
- }
}
if (esc) {
@@ -341,7 +337,7 @@
client->esc_state.str.str = esc;
}
- rc = client_init(client, console_id);
+ rc = client_init(client, config, console_id);
if (rc) {
goto out_config_fini;
}
diff --git a/console-server.c b/console-server.c
index 815431c..e60e280 100644
--- a/console-server.c
+++ b/console-server.c
@@ -391,28 +391,17 @@
return write_buf_to_fd(console->tty.fd, data, len);
}
-/* Read console if from config and prepare a socket name */
+/* Prepare a socket name */
static int set_socket_info(struct console *console, struct config *config,
const char *console_id)
{
- const char *resolved_id;
ssize_t len;
- if (console_id) {
- resolved_id = console_id;
- } else {
- resolved_id = config_get_value(config, "console-id");
-
- /* socket-id is deprecated */
- if (!resolved_id) {
- resolved_id = config_get_value(config, "socket-id");
- }
- }
-
- console->console_id = resolved_id;
+ /* Get console id */
+ console->console_id = config_resolve_console_id(config, console_id);
/* Get the socket name/path */
- len = console_socket_path(console->socket_name, resolved_id);
+ len = console_socket_path(console->socket_name, console->console_id);
if (len < 0) {
warn("Failed to set socket path: %s", strerror(errno));
return EXIT_FAILURE;
diff --git a/console-server.h b/console-server.h
index bd287c8..5343af8 100644
--- a/console-server.h
+++ b/console-server.h
@@ -209,6 +209,8 @@
struct config;
const char *config_get_value(struct config *config, const char *name);
struct config *config_init(const char *filename);
+const char *config_resolve_console_id(struct config *config,
+ const char *id_arg);
void config_fini(struct config *config);
int config_parse_baud(speed_t *speed, const char *baud_string);
diff --git a/console-socket.c b/console-socket.c
index d535182..3873440 100644
--- a/console-socket.c
+++ b/console-socket.c
@@ -34,7 +34,8 @@
ssize_t rc;
if (!id) {
- id = "default";
+ errno = EINVAL;
+ return -1;
}
rc = snprintf(sun_path + 1, sizeof(socket_path_t) - 1,
diff --git a/test/meson.build b/test/meson.build
index 149322f..0d08d19 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -2,6 +2,7 @@
'test-client-escape',
'test-config-parse',
'test-config-parse-logsize',
+ 'test-config-resolve-console-id',
'test-ringbuffer-boundary-poll',
'test-ringbuffer-boundary-read',
'test-ringbuffer-contained-offset-read',
diff --git a/test/test-config-resolve-console-id.c b/test/test-config-resolve-console-id.c
new file mode 100644
index 0000000..319f342
--- /dev/null
+++ b/test/test-config-resolve-console-id.c
@@ -0,0 +1,140 @@
+#include <assert.h>
+
+#define TEST_CONSOLE_ID "test"
+
+#include "config.c"
+
+static void test_independence_cmdline_optarg(void)
+{
+ const char *console_id;
+ struct config *ctx;
+
+ ctx = calloc(1, sizeof(*ctx));
+ console_id = config_resolve_console_id(ctx, TEST_CONSOLE_ID);
+
+ assert(!strcmp(console_id, TEST_CONSOLE_ID));
+
+ config_fini(ctx);
+}
+
+static void test_independence_config_console_id(void)
+{
+ const char *console_id;
+ struct config *ctx;
+ char *buf;
+
+ ctx = calloc(1, sizeof(*ctx));
+ buf = strdup("console-id = " TEST_CONSOLE_ID);
+ config_parse(ctx, buf);
+ free(buf);
+ console_id = config_resolve_console_id(ctx, NULL);
+
+ assert(!strcmp(console_id, TEST_CONSOLE_ID));
+
+ config_fini(ctx);
+}
+
+static void test_independence_config_socket_id(void)
+{
+ const char *console_id;
+ struct config *ctx;
+ char *buf;
+
+ ctx = calloc(1, sizeof(*ctx));
+ buf = strdup("socket-id = " TEST_CONSOLE_ID);
+ config_parse(ctx, buf);
+ free(buf);
+ console_id = config_resolve_console_id(ctx, NULL);
+
+ assert(!strcmp(console_id, TEST_CONSOLE_ID));
+
+ config_fini(ctx);
+}
+
+static void test_independence_default(void)
+{
+ const char *console_id;
+ struct config *ctx;
+
+ ctx = calloc(1, sizeof(*ctx));
+ console_id = config_resolve_console_id(ctx, NULL);
+
+ assert(!strcmp(console_id, DEFAULT_CONSOLE_ID));
+
+ config_fini(ctx);
+}
+
+static void test_precedence_cmdline_optarg(void)
+{
+ static const char *const config = "console-id = console\n"
+ "socket-id = socket\n";
+ const char *console_id;
+ struct config *ctx;
+ char *buf;
+
+ ctx = calloc(1, sizeof(*ctx));
+ buf = strdup(config);
+ config_parse(ctx, buf);
+ free(buf);
+ console_id = config_resolve_console_id(ctx, TEST_CONSOLE_ID);
+
+ assert(config_get_value(ctx, "console-id"));
+ assert(config_get_value(ctx, "socket-id"));
+ assert(!strcmp(console_id, TEST_CONSOLE_ID));
+
+ config_fini(ctx);
+}
+
+static void test_precedence_config_console_id(void)
+{
+ static const char *const config = "console-id = console\n"
+ "socket-id = socket\n";
+ const char *console_id;
+ struct config *ctx;
+ char *buf;
+
+ ctx = calloc(1, sizeof(*ctx));
+ buf = strdup(config);
+ config_parse(ctx, buf);
+ free(buf);
+ console_id = config_resolve_console_id(ctx, NULL);
+
+ assert(config_get_value(ctx, "console-id"));
+ assert(config_get_value(ctx, "socket-id"));
+ assert(!strcmp(console_id, "console"));
+
+ config_fini(ctx);
+}
+
+static void test_precedence_config_socket_id(void)
+{
+ static const char *const config = "socket-id = socket\n";
+ const char *console_id;
+ struct config *ctx;
+ char *buf;
+
+ ctx = calloc(1, sizeof(*ctx));
+ buf = strdup(config);
+ config_parse(ctx, buf);
+ free(buf);
+ console_id = config_resolve_console_id(ctx, NULL);
+
+ assert(!config_get_value(ctx, "console-id"));
+ assert(config_get_value(ctx, "socket-id"));
+ assert(!strcmp(console_id, "socket"));
+
+ config_fini(ctx);
+}
+
+int main(void)
+{
+ test_independence_cmdline_optarg();
+ test_independence_config_console_id();
+ test_independence_config_socket_id();
+ test_independence_default();
+ test_precedence_cmdline_optarg();
+ test_precedence_config_console_id();
+ test_precedence_config_socket_id();
+
+ return EXIT_SUCCESS;
+}