tray: better errors when parsing index.theme

swaybar/tray/icon.c

@@ -14,6 +14,10 @@
 #include "log.h"
 #include "stringop.h"
 
+static int cmp_id(const void *item, const void *cmp_to) {
+	return strcmp(item, cmp_to);
+}
+
 static bool dir_exists(char *path) {
 	struct stat sb;
 	return stat(path, &sb) == 0 && S_ISDIR(sb.st_mode);
@@ -78,35 +82,37 @@ static void destroy_theme(struct icon_theme *theme) {
 	free(theme);
 }
 
-static int cmp_group(const void *item, const void *cmp_to) {
+static const char *group_handler(char *old_group, char *new_group,
-	return strcmp(item, cmp_to);
-}
-
-static bool validate_icon_theme(struct icon_theme *theme) {
-	return theme && theme->name && theme->comment && theme->directories;
-}
-
-static bool group_handler(char *old_group, char *new_group,
 		struct icon_theme *theme) {
-	if (!old_group) { // first group must be "Icon Theme"
+	if (!old_group) {
-		if (!new_group) {
+		return new_group && strcmp(new_group, "Icon Theme") == 0 ? NULL :
-			return true;
+			"first group must be 'Icon Theme'";
-		}
-		return strcmp(new_group, "Icon Theme") != 0;
 	}
 
 	if (strcmp(old_group, "Icon Theme") == 0) {
-		if (!validate_icon_theme(theme)) {
+		if (!theme->name) {
-			return true;
+			return "missing required key 'Name'";
+		} else if (!theme->comment) {
+			return "missing required key 'Comment'";
+		} else if (!theme->directories) {
+			return "missing required key 'Directories'";
+		} else {
+			for (char *c = theme->name; *c; ++c) {
+				if (*c == ',' || *c == ' ') {
+					return "malformed theme name";
+				}
+			}
 		}
 	} else {
 		if (theme->subdirs->length == 0) { // skip
-			return false;
+			return NULL;
 		}
 
 		struct icon_theme_subdir *subdir =
 			theme->subdirs->items[theme->subdirs->length - 1];
-		if (!subdir->size) return true;
+		if (!subdir->size) {
+			return "missing required key 'Size'";
+		}
 
 		switch (subdir->type) {
 		case FIXED: subdir->max_size = subdir->min_size = subdir->size;
@@ -122,20 +128,20 @@ static bool group_handler(char *old_group, char *new_group,
 		}
 	}
 
-	if (new_group && list_seq_find(theme->directories, cmp_group, new_group) != -1) {
+	if (new_group && list_seq_find(theme->directories, cmp_id, new_group) != -1) {
 		struct icon_theme_subdir *subdir = calloc(1, sizeof(struct icon_theme_subdir));
 		if (!subdir) {
-			return true;
+			return "out of memory";
 		}
 		subdir->name = strdup(new_group);
 		subdir->threshold = 2;
 		list_add(theme->subdirs, subdir);
 	}
 
-	return false;
+	return NULL;
 }
 
-static int entry_handler(char *group, char *key, char *value,
+static const char *entry_handler(char *group, char *key, char *value,
 		struct icon_theme *theme) {
 	if (strcmp(group, "Icon Theme") == 0) {
 		if (strcmp(key, "Name") == 0) {
@@ -149,20 +155,17 @@ static int entry_handler(char *group, char *key, char *value,
 		} // Ignored: ScaledDirectories, Hidden, Example
 	} else {
 		if (theme->subdirs->length == 0) { // skip
-			return false;
+			return NULL;
 		}
 
 		struct icon_theme_subdir *subdir =
 			theme->subdirs->items[theme->subdirs->length - 1];
 		if (strcmp(subdir->name, group) != 0) { // skip
-			return false;
+			return NULL;
 		}
 
-		char *end;
+		if (strcmp(key, "Context") == 0) {
-		int n = strtol(value, &end, 10);
+			return NULL; // ignored, but explicitly handled to not fail parsing
-		if (strcmp(key, "Size") == 0) {
-			subdir->size = n;
-			return *end != '\0';
 		} else if (strcmp(key, "Type") == 0) {
 			if (strcmp(value, "Fixed") == 0) {
 				subdir->type = FIXED;
@@ -171,20 +174,28 @@ static int entry_handler(char *group, char *key, char *value,
 			} else if (strcmp(value, "Threshold") == 0) {
 				subdir->type = THRESHOLD;
 			} else {
-				return true;
+				return "invalid value - expected 'Fixed', 'Scalable' or 'Threshold'";
 			}
+			return NULL;
+		}
+
+		char *end;
+		int n = strtol(value, &end, 10);
+		if (*end != '\0') {
+			return "invalid value - expected a number";
+		}
+
+		if (strcmp(key, "Size") == 0) {
+			subdir->size = n;
 		} else if (strcmp(key, "MaxSize") == 0) {
 			subdir->max_size = n;
-			return *end != '\0';
 		} else if (strcmp(key, "MinSize") == 0) {
 			subdir->min_size = n;
-			return *end != '\0';
 		} else if (strcmp(key, "Threshold") == 0) {
 			subdir->threshold = n;
-			return *end != '\0';
+		} // Ignored: Scale
-		} // Ignored: Scale, Applications
 	}
-	return false;
+	return NULL;
 }
 
 /*
@@ -215,12 +226,16 @@ static struct icon_theme *read_theme_file(char *basedir, char *theme_name) {
 	}
 	theme->subdirs = create_list();
 
-	bool error = false;
+	list_t *groups = create_list();
-	char *group = NULL;
+
+	const char *error = NULL;
+	int line_no = 0;
 	char *full_line = NULL;
 	size_t full_len = 0;
 	ssize_t nread;
 	while ((nread = getline(&full_line, &full_len, theme_file)) != -1) {
+		++line_no;
+
 		char *line = full_line - 1;
 		while (isspace(*++line)) {} // remove leading whitespace
 		if (!*line || line[0] == '#') continue; // ignore blank lines & comments
@@ -231,37 +246,42 @@ static struct icon_theme *read_theme_file(char *basedir, char *theme_name) {
 
 		if (line[0] == '[') { // group header
 			// check well-formed
-			if (line[--len] != ']') {
-				error = true;
-				break;
-			}
 			int i = 1;
 			for (; !iscntrl(line[i]) && line[i] != '[' && line[i] != ']'; ++i) {}
-			if (i < len) {
+			if (i != --len || line[i] != ']') {
-				error = true;
+				error = "malformed group header";
+				break;
+			}
+
+			line[len] = '\0';
+
+			// check group is not duplicate
+			if (list_seq_find(groups, cmp_id, &line[1]) != -1) {
+				error = "duplicate group";
 				break;
 			}
 
 			// call handler
-			line[len] = '\0';
+			char *last_group = groups->length > 0 ? groups->items[groups->length - 1] : NULL;
-			error = group_handler(group, &line[1], theme);
+			error = group_handler(last_group, &line[1], theme);
 			if (error) {
 				break;
 			}
-			free(group);
+
-			group = strdup(&line[1]);
+			list_add(groups, strdup(&line[1]));
 		} else { // key-value pair
-			if (!group) {
+			if (groups->length == 0) {
-				error = true;
+				error = "unexpected content before first header";
 				break;
 			}
+
 			// check well-formed
 			int eok = 0;
 			for (; isalnum(line[eok]) || line[eok] == '-'; ++eok) {} // TODO locale?
 			int i = eok - 1;
 			while (isspace(line[++i])) {}
 			if (line[i] != '=') {
-				error = true;
+				error = "malformed key-value pair";
 				break;
 			}
 
@@ -269,28 +289,38 @@ static struct icon_theme *read_theme_file(char *basedir, char *theme_name) {
 			char *value = &line[i];
 			while (isspace(*++value)) {}
 			// TODO unescape value
-			error = entry_handler(group, line, value, theme);
+
+			error = entry_handler(groups->items[groups->length - 1], line,
+					value, theme);
 			if (error) {
 				break;
 			}
 		}
 	}
 
-	if (!error && group) {
+	if (!error) {
-		error = group_handler(group, NULL, theme);
+		if (groups->length > 0) {
+			error = group_handler(groups->items[groups->length - 1], NULL, theme);
+		} else {
+			error = "empty file";
+		}
 	}
 
-	free(group);
+	if (!error) {
-	free(full_line);
-	fclose(theme_file);
-
-	if (!error && validate_icon_theme(theme)) {
 		theme->dir = strdup(theme_name);
-		return theme;
 	} else {
+		char *last_group = groups->length > 0 ? groups->items[groups->length-1] : "n/a";
+		sway_log(SWAY_DEBUG, "Failed to load theme '%s' - parsing of file "
+				"'%s/%s/index.theme' failed on line %d (group '%s'): %s",
+				theme_name, basedir, theme_name, line_no, last_group, error);
 		destroy_theme(theme);
-		return NULL;
+		theme = NULL;
 	}
+
+	free(full_line);
+	list_free_items_and_destroy(groups);
+	fclose(theme_file);
+	return theme;
 }
 
 static list_t *load_themes_in_dir(char *basedir) {