sink.text.details: validate parameters using param-validation

This patch makes sink.text.details validate its parameters using the
param-validation utility, instead of doing it by hand.

To be able to re-use the parameter names in the statically defined
array, I had to change them to macros, to avoid this warning:

/home/smarchi/src/babeltrace/src/plugins/text/details/details.c:244:4: error: initializer element is not constant
  { color_param_name, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { BT_VALUE_TYPE_STRING, .string = {
    ^~~~~~~~~~~~~~~~

Change-Id: I4279348e8441c22afe1ae47a6650d0a118cfa42a
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2146
Tested-by: jenkins <jenkins@lttng.org>

diff --git a/src/plugins/text/Makefile.am b/src/plugins/text/Makefile.am
index bd74b0083c671934ee4a5fbb4e10ecb698d8028f..7ba0037177725d18ee74ee665e601b9580d2e4d9 100644 (file)
--- a/src/plugins/text/Makefile.am
+++ b/src/plugins/text/Makefile.am
@@ -18,5 +18,6 @@ babeltrace_plugin_text_la_LIBADD += \
        $(top_builddir)/src/lib/libbabeltrace2.la \
        $(top_builddir)/src/common/libbabeltrace2-common.la \
        $(top_builddir)/src/logging/libbabeltrace2-logging.la \
-       $(top_builddir)/src/compat/libcompat.la
+       $(top_builddir)/src/compat/libcompat.la \
+       $(top_builddir)/src/plugins/common/param-validation/libbabeltrace2-param-validation.la
 endif

diff --git a/src/plugins/text/details/details.c b/src/plugins/text/details/details.c
index 6c47fcfeb0f86ec5fa0d2cd7315a1abef408ec03..88b7fe326d5eaa533de6aabbfe693ddaf04dc814 100644 (file)
--- a/src/plugins/text/details/details.c
+++ b/src/plugins/text/details/details.c
@@ -31,6 +31,7 @@
 #include "common/assert.h"
 #include "details.h"
 #include "write.h"
+#include "plugins/common/param-validation/param-validation.h"
 
 #define LOG_WRONG_PARAM_TYPE(_name, _value, _exp_type)                 \
        do {                                                            \
@@ -41,35 +42,16 @@
                        bt_common_value_type_string(_exp_type));        \
        } while (0)
 
-static
-const char * const in_port_name = "in";
-
-static
-const char * const color_param_name = "color";
-
-static
-const char * const with_metadata_param_name = "with-metadata";
-
-static
-const char * const with_data_param_name = "with-data";
-
-static
-const char * const with_time_param_name = "with-time";
-
-static
-const char * const with_trace_name_param_name = "with-trace-name";
-
-static
-const char * const with_stream_class_name_param_name = "with-stream-class-name";
-
-static
-const char * const with_stream_name_param_name = "with-stream-name";
-
-static
-const char * const with_uuid_param_name = "with-uuid";
-
-static
-const char * const compact_param_name = "compact";
+#define IN_PORT_NAME "in"
+#define COLOR_PARAM_NAME "color"
+#define WITH_METADATA_PARAM_NAME "with-metadata"
+#define WITH_DATA_PARAM_NAME "with-data"
+#define WITH_TIME_PARAM_NAME "with-time"
+#define WITH_TRACE_NAME_PARAM_NAME "with-trace-name"
+#define WITH_STREAM_CLASS_NAME_PARAM_NAME "with-stream-class-name"
+#define WITH_STREAM_NAME_PARAM_NAME "with-stream-name"
+#define WITH_UUID_PARAM_NAME "with-uuid"
+#define COMPACT_PARAM_NAME "compact"
 
 BT_HIDDEN
 void details_destroy_details_trace_class_meta(
@@ -242,48 +224,63 @@ void details_finalize(bt_self_component_sink *comp)
 }
 
 static
-int configure_bool_opt(struct details_comp *details_comp,
+void configure_bool_opt(struct details_comp *details_comp,
                const bt_value *params, const char *param_name,
                bool default_value, bool *opt_value)
 {
-       int ret = 0;
        const bt_value *value;
 
        *opt_value = default_value;
        value = bt_value_map_borrow_entry_value_const(params, param_name);
        if (value) {
-               if (!bt_value_is_bool(value)) {
-                       LOG_WRONG_PARAM_TYPE(param_name, value,
-                               BT_VALUE_TYPE_BOOL);
-                       ret = -1;
-                       goto end;
-               }
-
                *opt_value = (bool) bt_value_bool_get(value);
        }
-
-end:
-       return ret;
 }
 
+static const char *color_choices[] = { "never", "auto", "always", NULL };
+
+static const struct bt_param_validation_map_value_entry_descr details_params[] = {
+       { COLOR_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { BT_VALUE_TYPE_STRING, .string = {
+               .choices = color_choices,
+       } } },
+       { WITH_METADATA_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { .type = BT_VALUE_TYPE_BOOL } },
+       { WITH_DATA_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { .type = BT_VALUE_TYPE_BOOL } },
+       { COMPACT_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { .type = BT_VALUE_TYPE_BOOL } },
+       { WITH_TIME_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { .type = BT_VALUE_TYPE_BOOL } },
+       { WITH_TRACE_NAME_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { .type = BT_VALUE_TYPE_BOOL } },
+       { WITH_STREAM_CLASS_NAME_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { .type = BT_VALUE_TYPE_BOOL } },
+       { WITH_STREAM_NAME_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { .type = BT_VALUE_TYPE_BOOL } },
+       { WITH_UUID_PARAM_NAME, BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_OPTIONAL, { .type = BT_VALUE_TYPE_BOOL } },
+       BT_PARAM_VALIDATION_MAP_VALUE_ENTRY_END
+};
+
 static
-int configure_details_comp(struct details_comp *details_comp,
+bt_component_class_initialize_method_status configure_details_comp(
+               struct details_comp *details_comp,
                const bt_value *params)
 {
-       int ret = 0;
+       bt_component_class_initialize_method_status status;
        const bt_value *value;
        const char *str;
+       enum bt_param_validation_status validation_status;
+       gchar *validate_error = NULL;
+
+       validation_status = bt_param_validation_validate(params,
+               details_params, &validate_error);
+       if (validation_status == BT_PARAM_VALIDATION_STATUS_MEMORY_ERROR) {
+               status = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_MEMORY_ERROR;
+               goto end;
+       } else if (validation_status == BT_PARAM_VALIDATION_STATUS_VALIDATION_ERROR) {
+               status = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_ERROR;
+               BT_COMP_LOGE_APPEND_CAUSE(details_comp->self_comp,
+                       "%s", validate_error);
+               goto end;
+       }
 
        /* Colorize output? */
        details_comp->cfg.with_color = bt_common_colors_supported();
-       value = bt_value_map_borrow_entry_value_const(params, color_param_name);
+       value = bt_value_map_borrow_entry_value_const(params, COLOR_PARAM_NAME);
        if (value) {
-               if (!bt_value_is_string(value)) {
-                       LOG_WRONG_PARAM_TYPE(color_param_name, value,
-                               BT_VALUE_TYPE_STRING);
-                       goto error;
-               }
-
                str = bt_value_string_get(value);
 
                if (strcmp(str, "never") == 0) {
@@ -291,82 +288,55 @@ int configure_details_comp(struct details_comp *details_comp,
                } else if (strcmp(str, "auto") == 0) {
                        details_comp->cfg.with_color =
                                bt_common_colors_supported();
-               } else if (strcmp(str, "always") == 0) {
-                       details_comp->cfg.with_color = true;
                } else {
-                       BT_COMP_LOGE("Invalid `%s` parameter: unknown value "
-                               "(expecting `never`, `auto`, or `always`): "
-                               "value=\"%s\"", color_param_name, str);
-                       goto error;
+                       BT_ASSERT(strcmp(str, "always") == 0);
+
+                       details_comp->cfg.with_color = true;
                }
        }
 
        /* With metadata objects? */
-       ret = configure_bool_opt(details_comp, params, with_metadata_param_name,
+       configure_bool_opt(details_comp, params, WITH_METADATA_PARAM_NAME,
                true, &details_comp->cfg.with_meta);
-       if (ret) {
-               goto error;
-       }
 
        /* With data objects? */
-       ret = configure_bool_opt(details_comp, params, with_data_param_name,
+       configure_bool_opt(details_comp, params, WITH_DATA_PARAM_NAME,
                true, &details_comp->cfg.with_data);
-       if (ret) {
-               goto error;
-       }
 
        /* Compact? */
-       ret = configure_bool_opt(details_comp, params, compact_param_name,
+       configure_bool_opt(details_comp, params, COMPACT_PARAM_NAME,
                false, &details_comp->cfg.compact);
-       if (ret) {
-               goto error;
-       }
 
        /* With time? */
-       ret = configure_bool_opt(details_comp, params, with_time_param_name,
+       configure_bool_opt(details_comp, params, WITH_TIME_PARAM_NAME,
                true, &details_comp->cfg.with_time);
-       if (ret) {
-               goto error;
-       }
 
        /* With trace name? */
-       ret = configure_bool_opt(details_comp, params,
-               with_trace_name_param_name,
+       configure_bool_opt(details_comp, params,
+               WITH_TRACE_NAME_PARAM_NAME,
                true, &details_comp->cfg.with_trace_name);
-       if (ret) {
-               goto error;
-       }
 
        /* With stream class name? */
-       ret = configure_bool_opt(details_comp, params,
-               with_stream_class_name_param_name,
+       configure_bool_opt(details_comp, params,
+               WITH_STREAM_CLASS_NAME_PARAM_NAME,
                true, &details_comp->cfg.with_stream_class_name);
-       if (ret) {
-               goto error;
-       }
 
        /* With stream name? */
-       ret = configure_bool_opt(details_comp, params,
-               with_stream_name_param_name,
+       configure_bool_opt(details_comp, params,
+               WITH_STREAM_NAME_PARAM_NAME,
                true, &details_comp->cfg.with_stream_name);
-       if (ret) {
-               goto error;
-       }
 
        /* With UUID? */
-       ret = configure_bool_opt(details_comp, params,
-               with_uuid_param_name, true, &details_comp->cfg.with_uuid);
-       if (ret) {
-               goto error;
-       }
+       configure_bool_opt(details_comp, params,
+               WITH_UUID_PARAM_NAME, true, &details_comp->cfg.with_uuid);
 
+       status = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_OK;
        goto end;
 
-error:
-       ret = -1;
-
 end:
-       return ret;
+       g_free(validate_error);
+
+       return status;
 }
 
 static
@@ -400,7 +370,7 @@ bt_component_class_initialize_method_status details_init(
        struct details_comp *details_comp = NULL;
 
        add_port_status = bt_self_component_sink_add_input_port(comp,
-               in_port_name, NULL, NULL);
+               IN_PORT_NAME, NULL, NULL);
        switch (add_port_status) {
        case BT_SELF_COMPONENT_ADD_PORT_STATUS_OK:
                status = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_OK;
@@ -457,18 +427,18 @@ details_graph_is_configured(bt_self_component_sink *comp)
                bt_self_component_sink_as_self_component(comp));
        BT_ASSERT(details_comp);
        in_port = bt_self_component_sink_borrow_input_port_by_name(comp,
-               in_port_name);
+               IN_PORT_NAME);
        if (!bt_port_is_connected(bt_port_input_as_port_const(
                        bt_self_component_port_input_as_port_input(in_port)))) {
                BT_COMP_LOGE("Single input port is not connected: "
-                       "port-name=\"%s\"", in_port_name);
+                       "port-name=\"%s\"", IN_PORT_NAME);
                status = BT_COMPONENT_CLASS_SINK_GRAPH_IS_CONFIGURED_METHOD_STATUS_ERROR;
                goto end;
        }
 
        msg_iter_status = bt_self_component_port_input_message_iterator_create_from_sink_component(
                comp, bt_self_component_sink_borrow_input_port_by_name(comp,
-                       in_port_name), &iterator);
+                       IN_PORT_NAME), &iterator);
        if (msg_iter_status != BT_SELF_COMPONENT_PORT_INPUT_MESSAGE_ITERATOR_CREATE_FROM_SINK_COMPONENT_STATUS_OK) {
                status = (int) msg_iter_status;
                goto end;