Re: [PATCH v3 10/12] plugins: add range list API

[Bernhard Beschow]

Am 1. März 2024 17:46:07 UTC schrieb Sven Schnelle <svens@stackframe.org>:
>Export range_list_from_string(), range_contains() and range_list_free()
>to allow plugins to parse filter ranges and match them to avoid
>reimplementing this functionality.
>
>Signed-off-by: Sven Schnelle <svens@stackframe.org>
>---
> include/qemu/qemu-plugin.h   | 41 ++++++++++++++++++++++++++++++++++++
> plugins/api.c                | 18 ++++++++++++++++
> plugins/qemu-plugins.symbols |  3 +++
> 3 files changed, 62 insertions(+)
>
>diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>index 5839feea4d..4910a63d70 100644
>--- a/include/qemu/qemu-plugin.h
>+++ b/include/qemu/qemu-plugin.h
>@@ -765,4 +765,45 @@ typedef struct Error Error;
> QEMU_PLUGIN_API
> void qemu_plugin_error_print(Error *err);
> 
>+typedef GList qemu_plugin_range_list;
>+
>+/**
>+ * qemu_plugin_ranges_from_string() - parse a filter range string
>+ *
>+ * @out_ranges: a pointer to a @qemu_plugin_range_list pointer
>+ * @filter_spec: input string
>+ * @errp: @Error string on parse failure
>+ *
>+ * This function parses a filter specification string and stores the
>+ * parsed adress ranges found in @out. On parse failure a @Error pointer
>+ * is stored in @errp. The function accepts a comma-separated list
>+ * of start and end addresses or single addresses.
>+ */
>+QEMU_PLUGIN_API
>+void qemu_plugin_range_list_from_string(qemu_plugin_range_list **out_range,

Nice series in general. One nitpick: When the API docs are generated I get the 
following error:

    include/qemu/qemu-plugin.h:788: warning: Function parameter or member 
'out_range' not described in 'qemu_plugin_range_list_from_string'

This is due to the parameter being documented as "out_ranges" which seems like 
the more appropriate name given its type. It might also be cleaner to have the 
same names in the source, too.

Best regards,
Bernhard

>+                                        const char *filter_spec,
>+                                        Error **errp);
>+
>+/**
>+ * qemu_plugin_range_list_contains() - match a value against a list
>+ * of ranges
>+ *
>+ * @ranges: a pointer to a @qemu_plugin_range_list
>+ * @val: the value to match
>+ *
>+ * This function matches @val against the adress range list in @ranges.
>+ * On success, true is returned, otherwise false.
>+ */
>+QEMU_PLUGIN_API
>+bool qemu_plugin_range_list_contains(qemu_plugin_range_list *ranges,
>+                                     uint64_t val);
>+
>+/**
>+ * qemu_plugin_range_list_free() - free a list of ranges
>+ *
>+ * @ranges: a pointer to the list to be freed
>+ */
>+QEMU_PLUGIN_API
>+void qemu_plugin_range_list_free(qemu_plugin_range_list *ranges);
>+
> #endif /* QEMU_QEMU_PLUGIN_H */
>diff --git a/plugins/api.c b/plugins/api.c
>index 8fd3a8964a..8dbd14c648 100644
>--- a/plugins/api.c
>+++ b/plugins/api.c
>@@ -472,3 +472,21 @@ void qemu_plugin_error_print(Error *err)
>     error_report_err(err);
> }
> 
>+void qemu_plugin_range_list_from_string(qemu_plugin_range_list **out,
>+                                        const char *filter_spec,
>+                                        Error **errp)
>+{
>+    return range_list_from_string(out, filter_spec, errp);
>+}
>+
>+bool qemu_plugin_range_list_contains(qemu_plugin_range_list *ranges,
>+                                     uint64_t val)
>+{
>+    return range_list_contains(ranges, val);
>+}
>+
>+void qemu_plugin_range_list_free(qemu_plugin_range_list *ranges)
>+{
>+    return range_list_free(ranges);
>+}
>+
>diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
>index b142d11e58..472b29fc5f 100644
>--- a/plugins/qemu-plugins.symbols
>+++ b/plugins/qemu-plugins.symbols
>@@ -21,6 +21,9 @@
>   qemu_plugin_num_vcpus;
>   qemu_plugin_outs;
>   qemu_plugin_path_to_binary;
>+  qemu_plugin_range_list_contains;
>+  qemu_plugin_range_list_free;
>+  qemu_plugin_range_list_from_string;
>   qemu_plugin_read_register;
>   qemu_plugin_register_atexit_cb;
>   qemu_plugin_register_flush_cb;