varlink: add concept for embedding comments into IDL structures

4 files changed, 204 insertions, 24 deletions

diff --git a/src/shared/varlink-idl.c b/src/shared/varlink-idl.c
index 5c0d720459..f249ff5955 100644
--- a/src/shared/varlink-idl.c
+++ b/src/shared/varlink-idl.c
@@ -5,6 +5,7 @@
 #include "set.h"
 #include "strv.h"
 #include "terminal-util.h"
+#include "utf8.h"
 #include "varlink-idl.h"
 
 #define DEPTH_MAX 64U
@@ -15,13 +16,63 @@ enum {
         COLOR_IDENTIFIER,
         COLOR_MARKS,         /* [], ->, ?, … */
         COLOR_RESET,
+        COLOR_COMMENT,
         _COLOR_MAX,
 };
 
 #define varlink_idl_log(error, format, ...) log_debug_errno(error, "Varlink-IDL: " format, ##__VA_ARGS__)
 #define varlink_idl_log_full(level, error, format, ...) log_full_errno(level, error, "Varlink-IDL: " format, ##__VA_ARGS__)
 
-static int varlink_idl_format_all_fields(FILE *f, const VarlinkSymbol *symbol, VarlinkFieldDirection direction, const char *indent, const char *const colors[static _COLOR_MAX]);
+static int varlink_idl_format_all_fields(FILE *f, const VarlinkSymbol *symbol, VarlinkFieldDirection direction, const char *indent, const char *const colors[static _COLOR_MAX], size_t cols);
+
+static int varlink_idl_format_comment(
+                FILE *f,
+                const char *text,
+                const char *indent,
+                const char *const colors[static _COLOR_MAX],
+                size_t cols) {
+
+        int r;
+
+        assert(f);
+        assert(colors);
+
+        if (!text) {
+                /* If text is NULL, output an empty but commented line */
+                fputs(strempty(indent), f);
+                fputs(colors[COLOR_COMMENT], f);
+                fputs("#", f);
+                fputs(colors[COLOR_RESET], f);
+                fputs("\n", f);
+                return 0;
+        }
+
+        _cleanup_strv_free_ char **l = NULL;
+        r = strv_split_full(&l, text, NEWLINE, 0);
+        if (r < 0)
+                return log_error_errno(r, "Failed to split comment string: %m");
+
+        size_t indent_width = utf8_console_width(indent);
+        size_t max_width = indent_width < cols ? cols - indent_width : 0;
+        if (max_width < 10)
+                max_width = 10;
+
+        _cleanup_strv_free_ char **broken = NULL;
+        r = strv_rebreak_lines(l, max_width, &broken);
+        if (r < 0)
+                return log_error_errno(r, "Failed to rebreak lines in comment: %m");
+
+        STRV_FOREACH(i, broken) {
+                fputs(strempty(indent), f);
+                fputs(colors[COLOR_COMMENT], f);
+                fputs("# ", f);
+                fputs(*i, f);
+                fputs(colors[COLOR_RESET], f);
+                fputs("\n", f);
+        }
+
+        return 0;
+}
 
 static int varlink_idl_format_enum_values(
                 FILE *f,
@@ -65,10 +116,12 @@ static int varlink_idl_format_field(
                 FILE *f,
                 const VarlinkField *field,
                 const char *indent,
-                const char *const colors[static _COLOR_MAX]) {
+                const char *const colors[static _COLOR_MAX],
+                size_t cols) {
 
         assert(f);
         assert(field);
+        assert(field->field_type != _VARLINK_FIELD_COMMENT);
 
         fputs(strempty(indent), f);
         fputs(colors[COLOR_IDENTIFIER], f);
@@ -146,7 +199,7 @@ static int varlink_idl_format_field(
                 break;
 
         case VARLINK_STRUCT:
-                return varlink_idl_format_all_fields(f, ASSERT_PTR(field->symbol), VARLINK_REGULAR, indent, colors);
+                return varlink_idl_format_all_fields(f, ASSERT_PTR(field->symbol), VARLINK_REGULAR, indent, colors, cols);
 
         case VARLINK_ENUM:
                 return varlink_idl_format_enum_values(f, ASSERT_PTR(field->symbol), indent, colors);
@@ -163,7 +216,8 @@ static int varlink_idl_format_all_fields(
                 const VarlinkSymbol *symbol,
                 VarlinkFieldDirection filter_direction,
                 const char *indent,
-                const char *const colors[static _COLOR_MAX]) {
+                const char *const colors[static _COLOR_MAX],
+                size_t cols) {
 
         _cleanup_free_ char *indent2 = NULL;
         bool first = true;
@@ -179,6 +233,9 @@ static int varlink_idl_format_all_fields(
 
         for (const VarlinkField *field = symbol->fields; field->field_type != _VARLINK_FIELD_TYPE_END_MARKER; field++) {
 
+                if (field->field_type == _VARLINK_FIELD_COMMENT) /* skip comments at first */
+                        continue;
+
                 if (field->field_direction != filter_direction)
                         continue;
 
@@ -188,7 +245,27 @@ static int varlink_idl_format_all_fields(
                 } else
                         fputs(",\n", f);
 
-                r = varlink_idl_format_field(f, field, indent2, colors);
+                /* We found a field we want to output. In this case, output all immediately preceeding
+                 * comments first. First, find the first comment in the series before. */
+                const VarlinkField *start_comment = NULL;
+                for (const VarlinkField *c1 = field; c1 > symbol->fields; c1--) {
+                        const VarlinkField *c0 = c1 - 1;
+
+                        if (c0->field_type != _VARLINK_FIELD_COMMENT)
+                                break;
+
+                        start_comment = c0;
+                }
+
+                if (start_comment) {
+                        for (const VarlinkField *c = start_comment; c < field; c++) {
+                                r = varlink_idl_format_comment(f, ASSERT_PTR(c->name), indent2, colors, cols);
+                                if (r < 0)
+                                        return r;
+                        }
+                }
+
+                r = varlink_idl_format_field(f, field, indent2, colors, cols);
                 if (r < 0)
                         return r;
         }
@@ -207,7 +284,8 @@ static int varlink_idl_format_all_fields(
 static int varlink_idl_format_symbol(
                 FILE *f,
                 const VarlinkSymbol *symbol,
-                const char *const colors[static _COLOR_MAX]) {
+                const char *const colors[static _COLOR_MAX],
+                size_t cols) {
         int r;
 
         assert(f);
@@ -232,7 +310,7 @@ static int varlink_idl_format_symbol(
                 fputs(symbol->name, f);
                 fputs(colors[COLOR_RESET], f);
 
-                r = varlink_idl_format_all_fields(f, symbol, VARLINK_REGULAR, /* indent= */ NULL, colors);
+                r = varlink_idl_format_all_fields(f, symbol, VARLINK_REGULAR, /* indent= */ NULL, colors, cols);
                 break;
 
         case VARLINK_METHOD:
@@ -242,7 +320,7 @@ static int varlink_idl_format_symbol(
                 fputs(symbol->name, f);
                 fputs(colors[COLOR_RESET], f);
 
-                r = varlink_idl_format_all_fields(f, symbol, VARLINK_INPUT, /* indent= */ NULL, colors);
+                r = varlink_idl_format_all_fields(f, symbol, VARLINK_INPUT, /* indent= */ NULL, colors, cols);
                 if (r < 0)
                         return r;
 
@@ -250,7 +328,7 @@ static int varlink_idl_format_symbol(
                 fputs(" -> ", f);
                 fputs(colors[COLOR_RESET], f);
 
-                r = varlink_idl_format_all_fields(f, symbol, VARLINK_OUTPUT, /* indent= */ NULL, colors);
+                r = varlink_idl_format_all_fields(f, symbol, VARLINK_OUTPUT, /* indent= */ NULL, colors, cols);
                 break;
 
         case VARLINK_ERROR:
@@ -260,7 +338,7 @@ static int varlink_idl_format_symbol(
                 fputs(symbol->name, f);
                 fputs(colors[COLOR_RESET], f);
 
-                r = varlink_idl_format_all_fields(f, symbol, VARLINK_REGULAR, /* indent= */ NULL, colors);
+                r = varlink_idl_format_all_fields(f, symbol, VARLINK_REGULAR, /* indent= */ NULL, colors, cols);
                 break;
 
         default:
@@ -277,7 +355,8 @@ static int varlink_idl_format_all_symbols(
                 FILE *f,
                 const VarlinkInterface *interface,
                 VarlinkSymbolType filter_type,
-                const char *const colors[static _COLOR_MAX]) {
+                const char *const colors[static _COLOR_MAX],
+                size_t cols) {
 
         int r;
 
@@ -289,9 +368,38 @@ static int varlink_idl_format_all_symbols(
                 if ((*symbol)->symbol_type != filter_type)
                         continue;
 
+                if ((*symbol)->symbol_type == _VARLINK_INTERFACE_COMMENT) {
+                        /* Interface comments we'll output directly. */
+                        r = varlink_idl_format_comment(f, ASSERT_PTR((*symbol)->name), /* indent= */ NULL, colors, cols);
+                        if (r < 0)
+                                return r;
+
+                        continue;
+                }
+
                 fputs("\n", f);
 
-                r = varlink_idl_format_symbol(f, *symbol, colors);
+                /* Symbol comments we'll only output if we are outputing the symbol they belong to. Scan
+                 * backwards for symbol comments. */
+                const VarlinkSymbol *const*start_comment = NULL;
+                for (const VarlinkSymbol *const*c1 = symbol; c1 > interface->symbols; c1--) {
+                        const VarlinkSymbol *const *c0 = c1 - 1;
+
+                        if ((*c0)->symbol_type != _VARLINK_SYMBOL_COMMENT)
+                                break;
+
+                        start_comment = c0;
+                }
+
+                /* Found one or more comments, output them now */
+                if (start_comment)
+                        for (const VarlinkSymbol *const*c = start_comment; c < symbol; c++) {
+                                r = varlink_idl_format_comment(f, ASSERT_PTR((*c)->name), /* indent= */ NULL, colors, cols);
+                                if (r < 0)
+                                        return r;
+                        }
+
+                r = varlink_idl_format_symbol(f, *symbol, colors, cols);
                 if (r < 0)
                         return r;
         }
@@ -299,17 +407,18 @@ static int varlink_idl_format_all_symbols(
         return 0;
 }
 
-int varlink_idl_dump(FILE *f, int use_colors, const VarlinkInterface *interface) {
+int varlink_idl_dump(FILE *f, int use_colors, size_t cols, const VarlinkInterface *interface) {
         static const char* const color_table[_COLOR_MAX] = {
                 [COLOR_SYMBOL_TYPE] = ANSI_HIGHLIGHT_GREEN,
                 [COLOR_FIELD_TYPE]  = ANSI_HIGHLIGHT_BLUE,
                 [COLOR_IDENTIFIER]  = ANSI_NORMAL,
                 [COLOR_MARKS]       = ANSI_HIGHLIGHT_MAGENTA,
                 [COLOR_RESET]       = ANSI_NORMAL,
+                [COLOR_COMMENT]     = ANSI_GREY,
         };
 
         static const char* const color_off[_COLOR_MAX] = {
-                "", "", "", "", "",
+                "", "", "", "", "", "",
         };
 
         int r;
@@ -324,6 +433,11 @@ int varlink_idl_dump(FILE *f, int use_colors, const VarlinkInterface *interface)
 
         const char *const *colors = use_colors ? color_table : color_off;
 
+        /* First output interface comments */
+        r = varlink_idl_format_all_symbols(f, interface, _VARLINK_INTERFACE_COMMENT, colors, cols);
+        if (r < 0)
+                return r;
+
         fputs(colors[COLOR_SYMBOL_TYPE], f);
         fputs("interface ", f);
         fputs(colors[COLOR_IDENTIFIER], f);
@@ -331,8 +445,15 @@ int varlink_idl_dump(FILE *f, int use_colors, const VarlinkInterface *interface)
         fputs(colors[COLOR_RESET], f);
         fputs("\n", f);
 
+        /* Then output all symbols, ordered by symbol type */
         for (VarlinkSymbolType t = 0; t < _VARLINK_SYMBOL_TYPE_MAX; t++) {
-                r = varlink_idl_format_all_symbols(f, interface, t, colors);
+
+                /* Interface comments we already have output above. Symbol comments are output when the
+                 * symbol they belong to are output, hence filter both here. */
+                if (IN_SET(t, _VARLINK_SYMBOL_COMMENT, _VARLINK_INTERFACE_COMMENT))
+                        continue;
+
+                r = varlink_idl_format_all_symbols(f, interface, t, colors, cols);
                 if (r < 0)
                         return r;
         }
@@ -340,14 +461,14 @@ int varlink_idl_dump(FILE *f, int use_colors, const VarlinkInterface *interface)
         return 0;
 }
 
-int varlink_idl_format(const VarlinkInterface *interface, char **ret) {
+int varlink_idl_format_full(const VarlinkInterface *interface, size_t cols, char **ret) {
         _cleanup_(memstream_done) MemStream memstream = {};
         int r;
 
         if (!memstream_init(&memstream))
                 return -errno;
 
-        r = varlink_idl_dump(memstream.f, /* use_colors= */ false, interface);
+        r = varlink_idl_dump(memstream.f, /* use_colors= */ false, cols, interface);
         if (r < 0)
                 return r;
 
@@ -1201,6 +1322,10 @@ bool varlink_idl_interface_name_is_valid(const char *name) {
         return true;
 }
 
+static bool varlink_idl_comment_is_valid(const char *comment) {
+        return utf8_is_valid(comment);
+}
+
 static int varlink_idl_symbol_consistent(const VarlinkInterface *interface, const VarlinkSymbol *symbol, int level);
 
 static int varlink_idl_field_consistent(
@@ -1293,6 +1418,9 @@ static int varlink_idl_field_consistent(
 static bool varlink_symbol_is_empty(const VarlinkSymbol *symbol) {
         assert(symbol);
 
+        if (IN_SET(symbol->symbol_type, _VARLINK_SYMBOL_COMMENT, _VARLINK_INTERFACE_COMMENT))
+                return true;
+
         return symbol->fields[0].field_type == _VARLINK_FIELD_TYPE_END_MARKER;
 }
 
@@ -1316,7 +1444,18 @@ static int varlink_idl_symbol_consistent(
         if (IN_SET(symbol->symbol_type, VARLINK_STRUCT_TYPE, VARLINK_ENUM_TYPE) && varlink_symbol_is_empty(symbol))
                 return varlink_idl_log_full(level, SYNTHETIC_ERRNO(EUCLEAN), "Symbol '%s' is empty, refusing.", symbol_name);
 
+        if (IN_SET(symbol->symbol_type, _VARLINK_SYMBOL_COMMENT, _VARLINK_INTERFACE_COMMENT))
+                return 0;
+
         for (const VarlinkField *field = symbol->fields; field->field_type != _VARLINK_FIELD_TYPE_END_MARKER; field++) {
+
+                if (field->field_type == _VARLINK_FIELD_COMMENT) {
+                        if (!varlink_idl_comment_is_valid(field->name))
+                                return varlink_idl_log_full(level, SYNTHETIC_ERRNO(EUCLEAN), "Comment in symbol '%s' not valid, refusing.", symbol_name);
+
+                        continue;
+                }
+
                 Set **name_set = field->field_direction == VARLINK_OUTPUT ? &output_set : &input_set; /* for the method case we need two separate sets, otherwise we use the same */
 
                 if (!varlink_idl_field_name_is_valid(field->name))
@@ -1347,6 +1486,12 @@ int varlink_idl_consistent(const VarlinkInterface *interface, int level) {
 
         for (const VarlinkSymbol *const *symbol = interface->symbols; *symbol; symbol++) {
 
+                if (IN_SET((*symbol)->symbol_type, _VARLINK_SYMBOL_COMMENT, _VARLINK_INTERFACE_COMMENT)) {
+                        if (!varlink_idl_comment_is_valid((*symbol)->name))
+                                return varlink_idl_log_full(level, SYNTHETIC_ERRNO(EUCLEAN), "Comment in interface '%s' not valid, refusing.", interface->name);
+                        continue;
+                }
+
                 if (!varlink_idl_symbol_name_is_valid((*symbol)->name))
                         return varlink_idl_log_full(level, SYNTHETIC_ERRNO(EUCLEAN), "Symbol name '%s' is not valid, refusing.", strempty((*symbol)->name));
 
@@ -1407,6 +1552,9 @@ static int varlink_idl_validate_field_element_type(const VarlinkField *field, sd
 
                 break;
 
+        case _VARLINK_FIELD_COMMENT:
+                break;
+
         default:
                 assert_not_reached();
         }
@@ -1418,6 +1566,7 @@ static int varlink_idl_validate_field(const VarlinkField *field, sd_json_variant
         int r;
 
         assert(field);
+        assert(field->field_type != _VARLINK_FIELD_COMMENT);
 
         if (!v || sd_json_variant_is_null(v)) {
 
@@ -1449,7 +1598,6 @@ static int varlink_idl_validate_field(const VarlinkField *field, sd_json_variant
                                 return r;
                 }
         } else {
-
                 r = varlink_idl_validate_field_element_type(field, v);
                 if (r < 0)
                         return r;
@@ -1462,6 +1610,7 @@ static int varlink_idl_validate_symbol(const VarlinkSymbol *symbol, sd_json_vari
         int r;
 
         assert(symbol);
+        assert(!IN_SET(symbol->symbol_type, _VARLINK_SYMBOL_COMMENT, _VARLINK_INTERFACE_COMMENT));
 
         if (!v) {
                 if (bad_field)
@@ -1537,6 +1686,10 @@ static int varlink_idl_validate_symbol(const VarlinkSymbol *symbol, sd_json_vari
                 break;
         }
 
+        case _VARLINK_SYMBOL_COMMENT:
+        case _VARLINK_INTERFACE_COMMENT:
+                break;
+
         default:
                 assert_not_reached();
         }
@@ -1579,6 +1732,7 @@ const VarlinkSymbol* varlink_idl_find_symbol(
 
         assert(interface);
         assert(type < _VARLINK_SYMBOL_TYPE_MAX);
+        assert(!IN_SET(type, _VARLINK_SYMBOL_COMMENT, _VARLINK_INTERFACE_COMMENT));
 
         if (isempty(name))
                 return NULL;
@@ -1603,9 +1757,13 @@ const VarlinkField* varlink_idl_find_field(
         if (isempty(name))
                 return NULL;
 
-        for (const VarlinkField *field = symbol->fields; field->field_type != _VARLINK_FIELD_TYPE_END_MARKER; field++)
+        for (const VarlinkField *field = symbol->fields; field->field_type != _VARLINK_FIELD_TYPE_END_MARKER; field++) {
+                if (field->field_type == _VARLINK_FIELD_COMMENT)
+                        continue;
+
                 if (streq_ptr(field->name, name))
                         return field;
+        }
 
         return NULL;
 }
diff --git a/src/shared/varlink-idl.h b/src/shared/varlink-idl.h
index 6d0b3490fb..03a1a2b2e4 100644
--- a/src/shared/varlink-idl.h
+++ b/src/shared/varlink-idl.h
@@ -20,6 +20,8 @@ typedef enum VarlinkSymbolType {
         VARLINK_STRUCT_TYPE,
         VARLINK_METHOD,
         VARLINK_ERROR,
+        _VARLINK_INTERFACE_COMMENT,     /* Not really a symbol, just a comment about the interface */
+        _VARLINK_SYMBOL_COMMENT,        /* Not really a symbol, just a comment about a symbol */
         _VARLINK_SYMBOL_TYPE_MAX,
         _VARLINK_SYMBOL_TYPE_INVALID = -EINVAL,
 } VarlinkSymbolType;
@@ -35,6 +37,7 @@ typedef enum VarlinkFieldType {
         VARLINK_STRING,
         VARLINK_OBJECT,
         VARLINK_ENUM_VALUE,
+        _VARLINK_FIELD_COMMENT,        /* Not really a field, just a comment about a field*/
         _VARLINK_FIELD_TYPE_MAX,
         _VARLINK_FIELD_TYPE_INVALID = -EINVAL,
 } VarlinkFieldType;
@@ -103,6 +106,9 @@ struct VarlinkInterface {
 #define VARLINK_DEFINE_ENUM_VALUE(_name)                                \
         { .name = #_name, .field_type = VARLINK_ENUM_VALUE }
 
+#define VARLINK_FIELD_COMMENT(text)                                     \
+        { .name = "" text, .field_type = _VARLINK_FIELD_COMMENT }
+
 #define VARLINK_DEFINE_METHOD(_name, ...)                               \
         const VarlinkSymbol vl_method_ ## _name = {                     \
                 .name = #_name,                                         \
@@ -137,8 +143,24 @@ struct VarlinkInterface {
                 .symbols = { __VA_ARGS__ __VA_OPT__(,) NULL},           \
         }
 
-int varlink_idl_dump(FILE *f, int use_colors, const VarlinkInterface *interface);
-int varlink_idl_format(const VarlinkInterface *interface, char **ret);
+#define VARLINK_SYMBOL_COMMENT(text)                                    \
+        &(const VarlinkSymbol) {                                        \
+                .name = "" text,                                        \
+                .symbol_type = _VARLINK_SYMBOL_COMMENT,                 \
+        }
+
+#define VARLINK_INTERFACE_COMMENT(text)                                 \
+        &(const VarlinkSymbol) {                                        \
+                .name = "" text,                                        \
+                .symbol_type = _VARLINK_INTERFACE_COMMENT,              \
+        }
+
+int varlink_idl_dump(FILE *f, int use_colors, size_t cols, const VarlinkInterface *interface);
+int varlink_idl_format_full(const VarlinkInterface *interface, size_t cols, char **ret);
+
+static inline int varlink_idl_format(const VarlinkInterface *interface, char **ret) {
+        return varlink_idl_format_full(interface, SIZE_MAX, ret);
+}
 
 int varlink_idl_parse(const char *text, unsigned *ret_line, unsigned *ret_column, VarlinkInterface **ret);
 VarlinkInterface* varlink_interface_free(VarlinkInterface *interface);
diff --git a/src/test/test-varlink-idl.c b/src/test/test-varlink-idl.c
index 34d84145fc..6a28edf467 100644
--- a/src/test/test-varlink-idl.c
+++ b/src/test/test-varlink-idl.c
@@ -117,7 +117,7 @@ static void test_parse_format_one(const VarlinkInterface *iface) {
 
         assert_se(iface);
 
-        assert_se(varlink_idl_dump(stdout, /* use_colors=*/ true, iface) >= 0);
+        assert_se(varlink_idl_dump(stdout, /* use_colors=*/ true, /* cols= */ SIZE_MAX, iface) >= 0);
         assert_se(varlink_idl_consistent(iface, LOG_ERR) >= 0);
         assert_se(varlink_idl_format(iface, &text) >= 0);
         assert_se(varlink_idl_parse(text, NULL, NULL, &parsed) >= 0);
diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c
index 3057c0542c..8752ee575c 100644
--- a/src/varlinkctl/varlinkctl.c
+++ b/src/varlinkctl/varlinkctl.c
@@ -392,7 +392,7 @@ static int verb_introspect(int argc, char *argv[], void *userdata) {
                                 }
                         } else {
                                 pager_open(arg_pager_flags);
-                                r = varlink_idl_dump(stdout, /* use_colors= */ -1, vi);
+                                r = varlink_idl_dump(stdout, /* use_colors= */ -1, on_tty() ? columns() : SIZE_MAX, vi);
                                 if (r < 0)
                                         return log_error_errno(r, "Failed to format parsed interface description: %m");
                         }
@@ -628,7 +628,7 @@ static int verb_validate_idl(int argc, char *argv[], void *userdata) {
 
         pager_open(arg_pager_flags);
 
-        r = varlink_idl_dump(stdout, /* use_colors= */ -1, vi);
+        r = varlink_idl_dump(stdout, /* use_colors= */ -1, on_tty() ? columns() : SIZE_MAX, vi);
         if (r < 0)
                 return log_error_errno(r, "Failed to format parsed interface description: %m");