From 34f714349be3bf5fd636cef7f824fa2350da94e9 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 19 May 2025 15:07:34 +0200 Subject: [PATCH 1/2] sd-varlink: Fix argument names of sd_varlink_idl_parse() --- src/libsystemd/sd-varlink/sd-varlink-idl.c | 46 +++++++++++----------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/libsystemd/sd-varlink/sd-varlink-idl.c b/src/libsystemd/sd-varlink/sd-varlink-idl.c index 3acb5a0eb6..0a57e2eb5d 100644 --- a/src/libsystemd/sd-varlink/sd-varlink-idl.c +++ b/src/libsystemd/sd-varlink/sd-varlink-idl.c @@ -1141,8 +1141,8 @@ static int varlink_idl_resolve_types(sd_varlink_interface *interface) { int varlink_idl_parse( const char *text, - unsigned *line, - unsigned *column, + unsigned *reterr_line, + unsigned *reterr_column, sd_varlink_interface **ret) { _cleanup_(varlink_interface_freep) sd_varlink_interface *interface = NULL; @@ -1166,18 +1166,18 @@ int varlink_idl_parse( const char **p = &text; int r; - if (!line) - line = &_line; - if (!column) - column = &_column; + if (!reterr_line) + reterr_line = &_line; + if (!reterr_column) + reterr_column = &_column; while (state != STATE_DONE) { _cleanup_free_ char *token = NULL; r = varlink_idl_subparse_token( p, - line, - column, + reterr_line, + reterr_column, allowed_delimiters, allowed_chars, &token); @@ -1188,11 +1188,11 @@ int varlink_idl_parse( case STATE_PRE_INTERFACE: if (!token) - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *reterr_line, *reterr_column); if (streq(token, "#")) { _cleanup_free_ char *comment = NULL; - r = varlink_idl_subparse_comment(&text, line, column, &comment); + r = varlink_idl_subparse_comment(&text, reterr_line, reterr_column, &comment); if (r < 0) return r; @@ -1213,12 +1213,12 @@ int varlink_idl_parse( allowed_delimiters = NULL; allowed_chars = VALID_CHARS_INTERFACE_NAME; } else - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *reterr_line, *reterr_column, token); break; case STATE_INTERFACE: if (!token) - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *reterr_line, *reterr_column); r = varlink_interface_realloc(&interface, n_symbols); if (r < 0) @@ -1241,7 +1241,7 @@ int varlink_idl_parse( if (streq(token, "#")) { _cleanup_free_ char *comment = NULL; - r = varlink_idl_subparse_comment(&text, line, column, &comment); + r = varlink_idl_subparse_comment(&text, reterr_line, reterr_column, &comment); if (r < 0) return r; @@ -1268,7 +1268,7 @@ int varlink_idl_parse( state = STATE_ERROR; allowed_chars = VALID_CHARS_IDENTIFIER; } else - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *reterr_line, *reterr_column, token); break; @@ -1277,7 +1277,7 @@ int varlink_idl_parse( n_fields = 0; if (!token) - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *reterr_line, *reterr_column); r = varlink_symbol_realloc(&symbol, n_fields); if (r < 0) @@ -1286,7 +1286,7 @@ int varlink_idl_parse( symbol->symbol_type = SD_VARLINK_METHOD; symbol->name = TAKE_PTR(token); - r = varlink_idl_subparse_struct_or_enum(&text, line, column, &symbol, &n_fields, SD_VARLINK_INPUT, 0); + r = varlink_idl_subparse_struct_or_enum(&text, reterr_line, reterr_column, &symbol, &n_fields, SD_VARLINK_INPUT, 0); if (r < 0) return r; @@ -1298,12 +1298,12 @@ int varlink_idl_parse( assert(symbol); if (!token) - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *reterr_line, *reterr_column); if (!streq(token, "->")) - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *line, *column, token); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Unexpected token '%s'.", *reterr_line, *reterr_column, token); - r = varlink_idl_subparse_struct_or_enum(&text, line, column, &symbol, &n_fields, SD_VARLINK_OUTPUT, 0); + r = varlink_idl_subparse_struct_or_enum(&text, reterr_line, reterr_column, &symbol, &n_fields, SD_VARLINK_OUTPUT, 0); if (r < 0) return r; @@ -1322,7 +1322,7 @@ int varlink_idl_parse( n_fields = 0; if (!token) - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *reterr_line, *reterr_column); r = varlink_symbol_realloc(&symbol, n_fields); if (r < 0) @@ -1331,7 +1331,7 @@ int varlink_idl_parse( symbol->symbol_type = _SD_VARLINK_SYMBOL_TYPE_INVALID; /* don't know yet if enum or struct, will be field in by varlink_idl_subparse_struct_or_enum() */ symbol->name = TAKE_PTR(token); - r = varlink_idl_subparse_struct_or_enum(&text, line, column, &symbol, &n_fields, SD_VARLINK_REGULAR, 0); + r = varlink_idl_subparse_struct_or_enum(&text, reterr_line, reterr_column, &symbol, &n_fields, SD_VARLINK_REGULAR, 0); if (r < 0) return r; @@ -1350,7 +1350,7 @@ int varlink_idl_parse( n_fields = 0; if (!token) - return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *line, *column); + return varlink_idl_log(SYNTHETIC_ERRNO(EBADMSG), "%u:%u: Premature EOF.", *reterr_line, *reterr_column); r = varlink_symbol_realloc(&symbol, n_fields); if (r < 0) @@ -1359,7 +1359,7 @@ int varlink_idl_parse( symbol->symbol_type = SD_VARLINK_ERROR; symbol->name = TAKE_PTR(token); - r = varlink_idl_subparse_struct_or_enum(&text, line, column, &symbol, &n_fields, SD_VARLINK_REGULAR, 0); + r = varlink_idl_subparse_struct_or_enum(&text, reterr_line, reterr_column, &symbol, &n_fields, SD_VARLINK_REGULAR, 0); if (r < 0) return r; From d94c24e9cc3adb03dab2878a6cbe61d483c3124e Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 19 May 2025 14:49:43 +0200 Subject: [PATCH 2/2] sd-varlink: Expose sd_varlink_idl_parse() We're planning to do code generation based on the systemd varlink APIs. To simplify this, let's expose the IDL parser, so we can use it to do code generation instead of having to write our own IDL parser. --- src/fuzz/fuzz-varlink-idl.c | 4 +-- src/libsystemd/libsystemd.sym | 2 ++ src/libsystemd/sd-varlink/sd-varlink-idl.c | 6 ++--- src/libsystemd/sd-varlink/varlink-idl-util.h | 4 --- src/systemd/sd-varlink-idl.h | 4 +++ src/test/test-varlink-idl.c | 28 ++++++++++---------- src/varlinkctl/varlinkctl.c | 8 +++--- 7 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/fuzz/fuzz-varlink-idl.c b/src/fuzz/fuzz-varlink-idl.c index 8981a363ec..9eb021a7d1 100644 --- a/src/fuzz/fuzz-varlink-idl.c +++ b/src/fuzz/fuzz-varlink-idl.c @@ -14,7 +14,7 @@ #include "varlink-idl-util.h" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { - _cleanup_(varlink_interface_freep) sd_varlink_interface *vi = NULL; + _cleanup_(sd_varlink_interface_freep) sd_varlink_interface *vi = NULL; _cleanup_free_ char *str = NULL, *dump = NULL; int r; @@ -25,7 +25,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { assert_se(str = memdup_suffix0(data, size)); - r = varlink_idl_parse(str, /* ret_line= */ NULL, /* column= */ NULL, &vi); + r = sd_varlink_idl_parse(str, /* ret_line= */ NULL, /* column= */ NULL, &vi); if (r < 0) { log_debug_errno(r, "Failed to parse varlink interface definition: %m"); return 0; diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 4543fb0e60..d16c7265f2 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -1072,6 +1072,8 @@ global: sd_varlink_get_input_fd; sd_varlink_get_n_fds; sd_varlink_get_output_fd; + sd_varlink_idl_parse; + sd_varlink_interface_free; sd_varlink_reset_fds; sd_varlink_server_listen_name; } LIBSYSTEMD_257; diff --git a/src/libsystemd/sd-varlink/sd-varlink-idl.c b/src/libsystemd/sd-varlink/sd-varlink-idl.c index 0a57e2eb5d..1184bd5951 100644 --- a/src/libsystemd/sd-varlink/sd-varlink-idl.c +++ b/src/libsystemd/sd-varlink/sd-varlink-idl.c @@ -588,7 +588,7 @@ static sd_varlink_symbol* varlink_symbol_free(sd_varlink_symbol *symbol) { return mfree(symbol); } -sd_varlink_interface* varlink_interface_free(sd_varlink_interface *interface) { +_public_ sd_varlink_interface* sd_varlink_interface_free(sd_varlink_interface *interface) { if (!interface) return NULL; @@ -1139,13 +1139,13 @@ static int varlink_idl_resolve_types(sd_varlink_interface *interface) { return 0; } -int varlink_idl_parse( +_public_ int sd_varlink_idl_parse( const char *text, unsigned *reterr_line, unsigned *reterr_column, sd_varlink_interface **ret) { - _cleanup_(varlink_interface_freep) sd_varlink_interface *interface = NULL; + _cleanup_(sd_varlink_interface_freep) sd_varlink_interface *interface = NULL; _cleanup_(varlink_symbol_freep) sd_varlink_symbol *symbol = NULL; enum { STATE_PRE_INTERFACE, diff --git a/src/libsystemd/sd-varlink/varlink-idl-util.h b/src/libsystemd/sd-varlink/varlink-idl-util.h index 9c5df26d12..990eb804a9 100644 --- a/src/libsystemd/sd-varlink/varlink-idl-util.h +++ b/src/libsystemd/sd-varlink/varlink-idl-util.h @@ -6,10 +6,6 @@ #include "memory-util.h" -int varlink_idl_parse(const char *text, unsigned *ret_line, unsigned *ret_column, sd_varlink_interface **ret); -sd_varlink_interface* varlink_interface_free(sd_varlink_interface *interface); -DEFINE_TRIVIAL_CLEANUP_FUNC(sd_varlink_interface*, varlink_interface_free); - bool varlink_idl_field_name_is_valid(const char *name); bool varlink_idl_symbol_name_is_valid(const char *name); bool varlink_idl_interface_name_is_valid(const char *name); diff --git a/src/systemd/sd-varlink-idl.h b/src/systemd/sd-varlink-idl.h index c580a0d990..0fe49b8677 100644 --- a/src/systemd/sd-varlink-idl.h +++ b/src/systemd/sd-varlink-idl.h @@ -221,6 +221,10 @@ int sd_varlink_idl_dump(FILE *f, const sd_varlink_interface *interface, sd_varli int sd_varlink_idl_format_full(const sd_varlink_interface *interface, sd_varlink_idl_format_flags_t flags, size_t cols, char **ret); int sd_varlink_idl_format(const sd_varlink_interface *interface, char **ret); +int sd_varlink_idl_parse(const char *text, unsigned *reterr_line, unsigned *reterr_column, sd_varlink_interface **ret); +sd_varlink_interface* sd_varlink_interface_free(sd_varlink_interface *interface); +_SD_DEFINE_POINTER_CLEANUP_FUNC(sd_varlink_interface, sd_varlink_interface_free); + _SD_END_DECLARATIONS; #endif diff --git a/src/test/test-varlink-idl.c b/src/test/test-varlink-idl.c index c2e8891edb..009d143fbc 100644 --- a/src/test/test-varlink-idl.c +++ b/src/test/test-varlink-idl.c @@ -129,7 +129,7 @@ static SD_VARLINK_DEFINE_INTERFACE( &vl_error_ErrorTest); static void test_parse_format_one(const sd_varlink_interface *iface) { - _cleanup_(varlink_interface_freep) sd_varlink_interface *parsed = NULL; + _cleanup_(sd_varlink_interface_freep) sd_varlink_interface *parsed = NULL; _cleanup_free_ char *text = NULL, *text2 = NULL; assert_se(iface); @@ -137,7 +137,7 @@ static void test_parse_format_one(const sd_varlink_interface *iface) { assert_se(sd_varlink_idl_dump(stdout, iface, SD_VARLINK_IDL_FORMAT_COLOR, /* cols= */ SIZE_MAX) >= 0); assert_se(varlink_idl_consistent(iface, LOG_ERR) >= 0); assert_se(sd_varlink_idl_format(iface, &text) >= 0); - assert_se(varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); + assert_se(sd_varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); assert_se(varlink_idl_consistent(parsed, LOG_ERR) >= 0); assert_se(sd_varlink_idl_format(parsed, &text2) >= 0); @@ -145,13 +145,13 @@ static void test_parse_format_one(const sd_varlink_interface *iface) { text = mfree(text); text2 = mfree(text2); - parsed = varlink_interface_free(parsed); + parsed = sd_varlink_interface_free(parsed); /* Do the same thing, but aggressively line break, and make sure this is roundtrippable as well */ assert_se(sd_varlink_idl_dump(stdout, iface, SD_VARLINK_IDL_FORMAT_COLOR, 23) >= 0); assert_se(varlink_idl_consistent(iface, LOG_ERR) >= 0); assert_se(sd_varlink_idl_format_full(iface, 0, 23, &text) >= 0); - assert_se(varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); + assert_se(sd_varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); assert_se(varlink_idl_consistent(parsed, LOG_ERR) >= 0); assert_se(sd_varlink_idl_format_full(parsed, 0, 23, &text2) >= 0); @@ -213,7 +213,7 @@ TEST(parse_format) { } TEST(parse) { - _cleanup_(varlink_interface_freep) sd_varlink_interface *parsed = NULL; + _cleanup_(sd_varlink_interface_freep) sd_varlink_interface *parsed = NULL; /* This one has (nested) enonymous enums and structs */ static const char text[] = @@ -222,13 +222,13 @@ TEST(parse) { "type Barstruct ( a : (x, y, z), b : (x : int), c: (f, ff, fff), d: object, e : (sub : (subsub: (subsubsub: string, subsubsub2: (iii, ooo)))))" ; - assert_se(varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); + assert_se(sd_varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); test_parse_format_one(parsed); - assert_se(varlink_idl_parse("interface org.freedesktop.Foo\n" - "type Foo (b: bool, c: foo, c: int)", NULL, NULL, NULL) == -ENETUNREACH); /* unresolved type */ - assert_se(varlink_idl_parse("interface org.freedesktop.Foo\n" - "type Foo ()", NULL, NULL, NULL) == -EBADMSG); /* empty struct/enum */ + assert_se(sd_varlink_idl_parse("interface org.freedesktop.Foo\n" + "type Foo (b: bool, c: foo, c: int)", NULL, NULL, NULL) == -ENETUNREACH); /* unresolved type */ + assert_se(sd_varlink_idl_parse("interface org.freedesktop.Foo\n" + "type Foo ()", NULL, NULL, NULL) == -EBADMSG); /* empty struct/enum */ } TEST(interface_name_is_valid) { @@ -300,7 +300,7 @@ TEST(qualified_symbol_name_is_valid) { TEST(validate_json) { - _cleanup_(varlink_interface_freep) sd_varlink_interface *parsed = NULL; + _cleanup_(sd_varlink_interface_freep) sd_varlink_interface *parsed = NULL; /* This one has (nested) enonymous enums and structs */ static const char text[] = @@ -311,7 +311,7 @@ TEST(validate_json) { "#paff\n" "b:int, c:?bool, d:[]int, e:?[string]bool, f:?(piff, paff), g:(f:float) ) -> ()\n"; - assert_se(varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); + assert_se(sd_varlink_idl_parse(text, NULL, NULL, &parsed) >= 0); test_parse_format_one(parsed); _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; @@ -330,7 +330,7 @@ TEST(validate_json) { } static int test_recursive_one(unsigned depth) { - _cleanup_(varlink_interface_freep) sd_varlink_interface *parsed = NULL; + _cleanup_(sd_varlink_interface_freep) sd_varlink_interface *parsed = NULL; _cleanup_free_ char *pre = NULL, *post = NULL, *text = NULL; static const char header[] = "interface recursive.test\n" @@ -346,7 +346,7 @@ static int test_recursive_one(unsigned depth) { if (!text) return log_oom(); - return varlink_idl_parse(text, NULL, NULL, &parsed); + return sd_varlink_idl_parse(text, NULL, NULL, &parsed); } TEST(recursive) { diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index 74f9151a4d..d34ef55cf8 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -449,7 +449,7 @@ static int verb_introspect(int argc, char *argv[], void *userdata) { { "description", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, 0, SD_JSON_MANDATORY }, {} }; - _cleanup_(varlink_interface_freep) sd_varlink_interface *vi = NULL; + _cleanup_(sd_varlink_interface_freep) sd_varlink_interface *vi = NULL; const char *description = NULL; unsigned line = 0, column = 0; @@ -461,7 +461,7 @@ static int verb_introspect(int argc, char *argv[], void *userdata) { print_separator(); /* Try to parse the returned description, so that we can add syntax highlighting */ - r = varlink_idl_parse(ASSERT_PTR(description), &line, &column, &vi); + r = sd_varlink_idl_parse(ASSERT_PTR(description), &line, &column, &vi); if (r < 0) { if (list_methods) return log_error_errno(r, "Failed to parse returned interface description at %u:%u: %m", line, column); @@ -818,7 +818,7 @@ static int verb_call(int argc, char *argv[], void *userdata) { } static int verb_validate_idl(int argc, char *argv[], void *userdata) { - _cleanup_(varlink_interface_freep) sd_varlink_interface *vi = NULL; + _cleanup_(sd_varlink_interface_freep) sd_varlink_interface *vi = NULL; _cleanup_free_ char *text = NULL; const char *fname; unsigned line = 1, column = 1; @@ -838,7 +838,7 @@ static int verb_validate_idl(int argc, char *argv[], void *userdata) { fname = ""; } - r = varlink_idl_parse(text, &line, &column, &vi); + r = sd_varlink_idl_parse(text, &line, &column, &vi); if (r == -EBADMSG) return log_error_errno(r, "%s:%u:%u: Bad syntax.", fname, line, column); if (r == -ENETUNREACH)