spa: Improve JSON error reporting

Add struct spa_error_location that holds information about some parsing
context such as the line and column number, error and line fragment
with the error.

Make spa_json_get_error() fill in the spa_error_location instead. Add
some error codes to the error state and use this to add a parsing reason
to the location.

Add a debug function to log the error location in a nice way. Also
add a FILE based debug context to log to any FILE.

Replace pw_properties_check_string() with
pw_properties_update_string_checked() and add
pw_properties_new_string_checked(). The check string behaviour can still
be done by setting props to NULL but the main purpose is to be able to
avoid parsing the json file twice in the future.

When using the old pw_properties_update_string(), log a warning to the
log when we fail to parse the complete string.

Use the new checked functions and the debug functions to report about
parsing errors in the tools and conf parsing.

This gives errors like:

```
> pw-loopback --playback-props '{ foo =  [ f : g ] }'
error: syntax error in --playback-props: Invalid array separator
line:      1 | { foo =  [ f : g ] }
col:      14 |              ^
```

src/pipewire/conf.c

@@ -27,6 +27,7 @@
 #include <spa/utils/result.h>
 #include <spa/utils/string.h>
 #include <spa/utils/json.h>
+#include <spa/debug/log.h>
 
 #include <pipewire/cleanup.h>
 #include <pipewire/impl.h>
@@ -384,10 +385,10 @@ int pw_conf_save_state(const char *prefix, const char *name, const struct pw_pro
 
 static int conf_load(const char *path, struct pw_properties *conf)
 {
-	char *data;
+	char *data = NULL;
 	struct stat sbuf;
 	int count;
-	int line = -1, col = -1;
+	struct spa_error_location loc = { 0 };
 	int res;
 
 	spa_autoclose int fd = open(path,  O_CLOEXEC | O_RDONLY);
@@ -401,12 +402,11 @@ static int conf_load(const char *path, struct pw_properties *conf)
 		if ((data = mmap(NULL, sbuf.st_size, PROT_READ, MAP_PRIVATE, fd, 0)) == MAP_FAILED)
 			goto error;
 
-		if (!pw_properties_check_string(data, sbuf.st_size, &line, &col)) {
+		count = pw_properties_update_string_checked(conf, data, sbuf.st_size, &loc);
+		if (count < 0) {
 			errno = EINVAL;
 			goto error;
 		}
-
-		count = pw_properties_update_string(conf, data, sbuf.st_size);
 		munmap(data, sbuf.st_size);
 	} else {
 		count = 0;
@@ -418,8 +418,9 @@ static int conf_load(const char *path, struct pw_properties *conf)
 
 error:
 	res = -errno;
-	if (line != -1)
-		pw_log_warn("%p: syntax error in config '%s': line:%d col:%d", conf, path, line, col);
+	if (loc.line != 0)
+		spa_debug_log_error_location(pw_log_get(), SPA_LOG_LEVEL_WARN, &loc,
+				"%p: error in config '%s': %s", conf, path, loc.reason);
 	else
 		pw_log_warn("%p: error loading config '%s': %m", conf, path);
 	return res;

src/pipewire/properties.c

@@ -9,6 +9,7 @@
 #include <spa/utils/json.h>
 #include <spa/utils/string.h>
 #include <spa/utils/cleanup.h>
+#include <spa/debug/log.h>
 
 #include "pipewire/array.h"
 #include "pipewire/log.h"
@@ -129,11 +130,14 @@ struct pw_properties *pw_properties_new_dict(const struct spa_dict *dict)
 	return &impl->this;
 }
 
-static bool update_string(struct pw_properties *props, const char *str, size_t size,
-		int *count, int *err_line, int *err_col)
+static int update_string(struct pw_properties *props, const char *str, size_t size,
+		int *count, struct spa_error_location *loc)
 {
 	struct spa_json it[2];
 	char key[1024];
+	struct spa_error_location el;
+	bool err;
+	int cnt = 0;
 
 	spa_json_init(&it[0], str, size);
 	if (spa_json_enter_object(&it[0], &it[1]) <= 0)
@@ -155,14 +159,29 @@ static bool update_string(struct pw_properties *props, const char *str, size_t s
 			if (len <= 0)
 				break;
 
-			if (props && (val = malloc(len+1)) != NULL)
+			if (props) {
+				if ((val = malloc(len+1)) == NULL) {
+					errno = ENOMEM;
+					it[1].state = SPA_JSON_ERROR_FLAG;
+					break;
+				}
 				spa_json_parse_stringn(value, len, val, len+1);
+			}
 		}
 		if (props)
-			*count += pw_properties_set(props, key, val);
+			cnt += pw_properties_set(props, key, val);
 	}
-
-	return !spa_json_get_error(&it[1], str, err_line, err_col);
+	if ((err = spa_json_get_error(&it[1], str, &el))) {
+		if (loc == NULL)
+			spa_debug_log_error_location(pw_log_get(), SPA_LOG_LEVEL_WARN,
+					&el, "error parsing more than %d properties: %s",
+					cnt, el.reason);
+		else
+			*loc = el;
+	}
+	if (count)
+		*count = cnt;
+	return !err;
 }
 
 /** Update the properties from the given string, overwriting any
@@ -177,22 +196,38 @@ SPA_EXPORT
 int pw_properties_update_string(struct pw_properties *props, const char *str, size_t size)
 {
 	int count = 0;
-
-	update_string(props, str, size, &count, NULL, NULL);
+	update_string(props, str, size, &count, NULL);
 	return count;
 }
 
-/** Check \a str is a well-formed properties JSON string.
+/** Check \a str is a well-formed properties JSON string and update
+ * the properties on success.
  *
- * \param line  Return value for parse error line position
- * \param col  Return value for parse error column position
- * \return true if string is valid
+ * \a str should be a whitespace separated list of key=value
+ * strings or a json object, see pw_properties_new_string().
+ *
+ * When the check fails, this function will not update \a props.
+ *
+ * \param props  The properties to attempt to update, maybe be NULL
+ *            to simply check the JSON string.
+ * \param str  The JSON object with new values
+ * \param size  The length of the JSON string.
+ * \param loc  Return value for parse error location
+ * \return a negative value when string is not valid and \a loc contains
+ *         the error location or the number of updated properties in
+ *         \a props.
  * \since 1.1.0
  */
 SPA_EXPORT
-bool pw_properties_check_string(const char *str, size_t size, int *line, int *col)
+int pw_properties_update_string_checked(struct pw_properties *props,
+		const char *str, size_t size, struct spa_error_location *loc)
 {
-	return update_string(NULL, str, size, NULL, line, col);
+	int count = 0;
+	if (!update_string(NULL, str, size, NULL, loc))
+		return -EINVAL;
+	if (props)
+		update_string(props, str, size, &count, NULL);
+	return count;
 }
 
 /** Make a new properties object from the given str
@@ -224,6 +259,27 @@ error:
 	return NULL;
 }
 
+SPA_EXPORT
+struct pw_properties *
+pw_properties_new_string_checked(const char *object, size_t size, struct spa_error_location *loc)
+{
+	struct properties *impl;
+	int res;
+
+	impl = properties_new(16);
+	if (impl == NULL)
+		return NULL;
+
+	if ((res = pw_properties_update_string_checked(&impl->this, object, size, loc)) < 0)
+		goto error;
+
+	return &impl->this;
+error:
+	pw_properties_free(&impl->this);
+	errno = -res;
+	return NULL;
+}
+
 /** Copy a properties object
  *
  * \param properties properties to copy

src/pipewire/properties.h

@@ -40,6 +40,10 @@ pw_properties_new_dict(const struct spa_dict *dict);
 struct pw_properties *
 pw_properties_new_string(const char *args);
 
+struct pw_properties *
+pw_properties_new_string_checked(const char *args, size_t size,
+		struct spa_error_location *loc);
+
 struct pw_properties *
 pw_properties_copy(const struct pw_properties *properties);
 
@@ -51,11 +55,13 @@ int pw_properties_update_ignore(struct pw_properties *props,
 /* Update props with all key/value pairs from dict */
 int pw_properties_update(struct pw_properties *props,
 		     const struct spa_dict *dict);
+
 /* Update props with all key/value pairs from str */
 int pw_properties_update_string(struct pw_properties *props,
 		const char *str, size_t size);
 
-bool pw_properties_check_string(const char *str, size_t size, int *line, int *col);
+int pw_properties_update_string_checked(struct pw_properties *props,
+		const char *str, size_t size, struct spa_error_location *loc);
 
 int pw_properties_add(struct pw_properties *oldprops,
 		     const struct spa_dict *dict);

src/tools/pw-cat.c

@@ -26,6 +26,7 @@
 #include <spa/utils/string.h>
 #include <spa/utils/json.h>
 #include <spa/debug/types.h>
+#include <spa/debug/file.h>
 
 #include <pipewire/cleanup.h>
 #include <pipewire/pipewire.h>
@@ -1606,8 +1607,9 @@ int main(int argc, char *argv[])
 	uint8_t buffer[1024];
 	struct spa_pod_builder b = SPA_POD_BUILDER_INIT(buffer, sizeof(buffer));
 	const char *prog;
-	int exit_code = EXIT_FAILURE, c, ret, line, col;
+	int exit_code = EXIT_FAILURE, c, ret;
 	enum pw_stream_flags flags = 0;
+	struct spa_error_location loc;
 
 	setlocale(LC_ALL, "");
 	pw_init(&argc, &argv);
@@ -1729,11 +1731,12 @@ int main(int argc, char *argv[])
 			break;
 
 		case 'P':
-			if (!pw_properties_check_string(optarg, strlen(optarg), &line, &col)) {
-				fprintf(stderr, "error: syntax error in --properties at line:%d col:%d\n", line, col);
+			if (pw_properties_update_string_checked(data.props, optarg, strlen(optarg), &loc) < 0) {
+				spa_debug_file_error_location(stderr, &loc,
+						"error: syntax error in --properties: %s",
+						loc.reason);
 				goto error_usage;
 			}
-			pw_properties_update_string(data.props, optarg, strlen(optarg));
 			break;
 
 		case OPT_TARGET:

src/tools/pw-cli.c

@@ -1405,17 +1405,13 @@ static bool do_info(struct data *data, const char *cmd, char *args, char **error
 static struct pw_properties *properties_new_checked(const char *str, char **error)
 {
 	struct pw_properties *props;
-	int line, col;
+	struct spa_error_location loc;
 
-	if (!pw_properties_check_string(str, strlen(str), &line, &col)) {
-		*error = spa_aprintf("syntax error in properties, line:%d col:%d", line, col);
-		return NULL;
+	props = pw_properties_new_string_checked(str, strlen(str), &loc);
+	if (!props) {
+		*error = spa_aprintf("syntax error in properties, line:%d col:%d: %s",
+				loc.line, loc.col, loc.reason);
 	}
-
-	props = pw_properties_new_string(str);
-	if (!props)
-		*error = spa_aprintf("failed to allocate properties");
-
 	return props;
 }
 

src/tools/pw-container.c

@@ -18,6 +18,7 @@
 #include <spa/debug/pod.h>
 #include <spa/debug/format.h>
 #include <spa/debug/types.h>
+#include <spa/debug/file.h>
 
 #include <pipewire/pipewire.h>
 #include <pipewire/extensions/security-context.h>
@@ -147,7 +148,8 @@ int main(int argc, char *argv[])
 		{ "properties",		required_argument,	NULL, 'P' },
 		{ NULL,	0, NULL, 0}
 	};
-	int c, res, listen_fd, close_fd[2], line, col;
+	struct spa_error_location loc;
+	int c, res, listen_fd, close_fd[2];
 	char temp[PATH_MAX] = "/tmp/pipewire-XXXXXX";
 	struct sockaddr_un sockaddr = {0};
 
@@ -176,11 +178,12 @@ int main(int argc, char *argv[])
 			opt_remote = optarg;
 			break;
 		case 'P':
-			if (!pw_properties_check_string(optarg, strlen(optarg), &line, &col)) {
-				fprintf(stderr, "error: syntax error in --properties at line:%d col:%d\n", line, col);
+			if (pw_properties_update_string_checked(data.props, optarg, strlen(optarg), &loc) < 0) {
+				spa_debug_file_error_location(stderr, &loc,
+						"error: syntax error in --properties: %s",
+						loc.reason);
 				return -1;
 			}
-			pw_properties_update_string(data.props, optarg, strlen(optarg));
 			break;
 		default:
 			show_help(&data, argv[0], true);

src/tools/pw-dot.c

@@ -16,6 +16,7 @@
 #include <spa/utils/string.h>
 #include <spa/utils/json.h>
 #include <spa/debug/types.h>
+#include <spa/debug/file.h>
 
 #include <pipewire/pipewire.h>
 
@@ -1277,7 +1278,7 @@ static int get_data_from_json(struct data *data, const char *json_path)
 	struct stat sbuf;
 	struct spa_json it[2];
 	const char *value;
-	int line, col;
+	struct spa_error_location loc;
 
 	if ((fd = open(json_path,  O_CLOEXEC | O_RDONLY)) < 0) {
 		fprintf(stderr, "error opening file '%s': %m\n", json_path);
@@ -1314,8 +1315,10 @@ static int get_data_from_json(struct data *data, const char *json_path)
 
 	munmap(json, sbuf.st_size);
 
-	if (spa_json_get_error(&it[0], json, &line, &col)) {
-		fprintf(stderr, "JSON syntax error on line:%d col:%d\n", line, col);
+	if (spa_json_get_error(&it[0], json, &loc)) {
+		spa_debug_file_error_location(stderr, &loc,
+				"JSON syntax error: %s\n",
+				loc.reason);
 		return -1;
 	}
 

src/tools/pw-link.c

@@ -13,6 +13,7 @@
 #include <spa/utils/result.h>
 #include <spa/utils/string.h>
 #include <spa/utils/defs.h>
+#include <spa/debug/file.h>
 
 #include <pipewire/pipewire.h>
 #include <pipewire/filter.h>
@@ -873,7 +874,8 @@ static int run(int argc, char *argv[])
 		.objects = SPA_LIST_INIT(&data.objects),
 		.target_links = SPA_LIST_INIT(&data.target_links),
 	};
-	int res = 0, c, line, col;
+	int res = 0, c;
+	struct spa_error_location loc;
 	static const struct option long_options[] = {
 		{ "help",	no_argument,		NULL, 'h' },
 		{ "version",	no_argument,		NULL, 'V' },
@@ -942,11 +944,11 @@ static int run(int argc, char *argv[])
 			pw_properties_set(data.props, PW_KEY_LINK_PASSIVE, "true");
 			break;
 		case 'p':
-			if (!pw_properties_check_string(optarg, strlen(optarg), &line, &col)) {
-				fprintf(stderr, "error: syntax error in --props at line:%d col:%d\n", line, col);
+			if (pw_properties_update_string_checked(data.props, optarg, strlen(optarg), &loc) < 0) {
+				spa_debug_file_error_location(stderr, &loc,
+						"error: syntax error in --props: %s", loc.reason);
 				return -1;
 			}
-			pw_properties_update_string(data.props, optarg, strlen(optarg));
 			break;
 		case 'd':
 			data.opt_mode = MODE_DISCONNECT;

src/tools/pw-loopback.c

@@ -16,6 +16,7 @@
 #include <spa/param/audio/format-utils.h>
 #include <spa/param/audio/raw.h>
 #include <spa/utils/json.h>
+#include <spa/debug/file.h>
 
 #include <pipewire/pipewire.h>
 #include <pipewire/impl.h>
@@ -110,7 +111,8 @@ int main(int argc, char *argv[])
 		{ "playback-props",	required_argument,	NULL, 'o' },
 		{ NULL, 0, NULL, 0}
 	};
-	int c, res = -1, line, col;
+	int c, res = -1;
+	struct spa_error_location loc;
 
 	setlocale(LC_ALL, "");
 	pw_init(&argc, &argv);
@@ -170,18 +172,22 @@ int main(int argc, char *argv[])
 			pw_properties_set(data.playback_props, PW_KEY_TARGET_OBJECT, optarg);
 			break;
 		case 'i':
-			if (!pw_properties_check_string(optarg, strlen(optarg), &line, &col)) {
-				fprintf(stderr, "error: syntax error in --capture-props at line:%d col:%d\n", line, col);
+			if (pw_properties_update_string_checked(data.capture_props,
+						optarg, strlen(optarg), &loc) < 0) {
+				spa_debug_file_error_location(stderr, &loc,
+						"error: syntax error in --capture-props: %s",
+						loc.reason);
 				return -1;
 			}
-			pw_properties_update_string(data.capture_props, optarg, strlen(optarg));
 			break;
 		case 'o':
-			if (!pw_properties_check_string(optarg, strlen(optarg), &line, &col)) {
-				fprintf(stderr, "error: syntax error in --playback-props at line:%d col:%d\n", line, col);
+			if (pw_properties_update_string_checked(data.playback_props,
+						optarg, strlen(optarg), &loc) < 0) {
+				spa_debug_file_error_location(stderr, &loc,
+						"error: syntax error in --playback-props: %s",
+						loc.reason);
 				return -1;
 			}
-			pw_properties_update_string(data.playback_props, optarg, strlen(optarg));
 			break;
 		default:
 			show_help(&data, argv[0], true);