From: Gerhard Sittig <redacted>
Date: Fri, 29 May 2020 08:07:05 +0000 (+0200)
Subject: parsers: warn about unknown input/output module option keys
X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=cfad6a304754649dbd8841af8a15ddba16a69cc6;p=sigrok-cli.git

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 7d3396f..0c8d486 100644
--- 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 5ec655f..5652b78 100644
--- 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 cb8a1b4..b3d39ff 100644
--- 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 40015fd..a3905fd 100644
--- 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);