parsers: warn about unknown input/output module option keys

Misspelled option names fail silently, the setting won't take effect and
the user won't notice.

  $ echo "1, 0" | sigrok-cli -I csv:header=no:column_format=t,l -i -

Check whether specified input/output module options are supported, emit
warnings when they are not. It's a design choice to raise awareness yet
make mismatches non-fatal. This can be useful for generated command lines
or during a period of migration after UI changes.

The implementation is prepared for optional call site specific captions in
diagnostics messages. Callers can either have messages generated by common
code, or handle the list of unavailable words themselves, or have common
code emit warnings and in addition take extra actions. It's also trivial
to make mismatches fatal should the need arise.

This resolves the sigrok-cli part of bug #1549. Also addresses brace style
nits in bypassing.

diff --git a/input.c b/input.c
index 7d3396fc744503cdad7e348d2eb319a98e584556..0c8d48670ac8f17ea5e9e666c75a1dfe4160dfbf 100644 (file)
--- a/input.c
+++ b/input.c
@@ -65,9 +65,11 @@ static void load_input_file_module(struct df_arg_desc *df_arg)
                g_hash_table_remove(mod_args, "sigrok_key");
                if ((options = sr_input_options_get(imod))) {
                        mod_opts = generic_arg_to_opt(options, mod_args);
+                       (void)warn_unknown_keys(options, mod_args, NULL);
                        sr_output_options_free(options);
-               } else
+               } else {
                        mod_opts = NULL;
+               }
                if (!(in = sr_input_new(imod, mod_opts)))
                        g_critical("Error: failed to initialize input module.");
                if (mod_opts)

diff --git a/parsers.c b/parsers.c
index 5ec655f78ad3dce8a05f01186a2a7ec61febb437..5652b78dc096eaf3ef95d43e63e61a6f656eb891 100644 (file)
--- a/parsers.c
+++ b/parsers.c
@@ -297,6 +297,57 @@ GHashTable *parse_generic_arg(const char *arg, gboolean sep_first)
        return hash;
 }
 
+GSList *check_unknown_keys(const struct sr_option **avail, GHashTable *used)
+{
+       GSList *unknown;
+       GHashTableIter iter;
+       void *key;
+       const char *used_id;
+       size_t avail_idx;
+       const char *avail_id, *found_id;
+
+       /* Collect a list of used but not available keywords. */
+       unknown = NULL;
+       g_hash_table_iter_init(&iter, used);
+       while (g_hash_table_iter_next(&iter, &key, NULL)) {
+               used_id = key;
+               found_id = NULL;
+               for (avail_idx = 0; avail[avail_idx] && avail[avail_idx]->id; avail_idx++) {
+                       avail_id = avail[avail_idx]->id;
+                       if (strcmp(avail_id, used_id) == 0) {
+                               found_id = avail_id;
+                               break;
+                       }
+               }
+               if (!found_id)
+                       unknown = g_slist_append(unknown, g_strdup(used_id));
+       }
+
+       /* Return the list of unknown keywords, or NULL if empty. */
+       return unknown;
+}
+
+gboolean warn_unknown_keys(const struct sr_option **avail, GHashTable *used,
+       const char *caption)
+{
+       GSList *unknown, *l;
+       gboolean had_unknown;
+       const char *s;
+
+       if (!caption || !*caption)
+               caption = "Unknown keyword";
+
+       unknown = check_unknown_keys(avail, used);
+       had_unknown = unknown != NULL;
+       for (l = unknown; l; l = l->next) {
+               s = l->data;
+               g_warning("%s: %s.", caption, s);
+       }
+       g_slist_free_full(unknown, g_free);
+
+       return had_unknown;
+}
+
 GHashTable *generic_arg_to_opt(const struct sr_option **opts, GHashTable *genargs)
 {
        GHashTable *hash;

diff --git a/session.c b/session.c
index cb8a1b4f27ab742f796d46a7e9cc2e47c40088c9..b3d39ff8ac1f35d58dc978ed73caab41898794fc 100644 (file)
--- a/session.c
+++ b/session.c
@@ -101,9 +101,11 @@ const struct sr_output *setup_output_format(const struct sr_dev_inst *sdi, FILE
        g_hash_table_remove(fmtargs, "sigrok_key");
        if ((options = sr_output_options_get(omod))) {
                fmtopts = generic_arg_to_opt(options, fmtargs);
+               (void)warn_unknown_keys(options, fmtargs, NULL);
                sr_output_options_free(options);
-       } else
+       } else {
                fmtopts = NULL;
+       }
        o = sr_output_new(omod, fmtopts, sdi, opt_output_file);
 
        if (opt_output_file) {
@@ -145,9 +147,11 @@ const struct sr_transform *setup_transform_module(const struct sr_dev_inst *sdi)
        g_hash_table_remove(fmtargs, "sigrok_key");
        if ((options = sr_transform_options_get(tmod))) {
                fmtopts = generic_arg_to_opt(options, fmtargs);
+               (void)warn_unknown_keys(options, fmtargs, NULL);
                sr_transform_options_free(options);
-       } else
+       } else {
                fmtopts = NULL;
+       }
        t = sr_transform_new(tmod, fmtopts, sdi);
        if (fmtopts)
                g_hash_table_destroy(fmtopts);

diff --git a/sigrok-cli.h b/sigrok-cli.h
index 40015fd2c106ede81243095008b385617cd6fe58..a3905fd3b841e4e5416ae0c55e8c17e7876515b7 100644 (file)
--- a/sigrok-cli.h
+++ b/sigrok-cli.h
@@ -107,6 +107,9 @@ int parse_triggerstring(const struct sr_dev_inst *sdi, const char *s,
                struct sr_trigger **trigger);
 GHashTable *parse_generic_arg(const char *arg, gboolean sep_first);
 GHashTable *generic_arg_to_opt(const struct sr_option **opts, GHashTable *genargs);
+GSList *check_unknown_keys(const struct sr_option **avail, GHashTable *used);
+gboolean warn_unknown_keys(const struct sr_option **avail, GHashTable *used,
+               const char *caption);
 int canon_cmp(const char *str1, const char *str2);
 int parse_driver(char *arg, struct sr_dev_driver **driver, GSList **drvopts);