On 2023/11/09 17:50, Landry Breuil wrote:
> now with 100%+ more parse_line_like_wordexp_does() manually rolled
> (thanks tb!)... and 100%+ more malloc return values checked. with that
> horror i'm able to run swayidle with this in .config/swayidle/config:
The bits that I've looked at seem a lot better. Some small feedback,
static char *get_config_path(void) {
+#if HAVE_WORDEXP
static char *config_paths[3] = {
"$XDG_CONFIG_HOME/swayidle/config",
"$HOME/.swayidle/config",
@@ -999,10 +1004,123 @@ static char *get_config_path(void) {
free(path);
}
}
+#else
TBH I'd just delete the wordexp version of this part, there seems no
benefit to using wordexp here. Unless being able to execute backtick
commands embedded in XDG_CONFIG_HOME is considered a useful feature :)
+ char *home = getenv("HOME");
+ char *path;
+ int n, len;
+ if (home) {
+ len = strlen(home) + strlen("/.swayidle/config") + 1;
+ path = malloc(len);
+ if (path == NULL)
+ return NULL;
+ n = snprintf(path, len, "%s/.swayidle/config", home);
+ if (n < len && access(path, R_OK) == 0)
+ return path;
+ free(path);
+ char *config_home = getenv("XDG_CONFIG_HOME");
+ if (!config_home || config_home[0] == '\0') {
+ len = strlen(home) + strlen("/.config/swayidle/config")
+ 1;
+ path = malloc(len);
[..]
I don't think it's worth sizing the variable so carefully. You can
just malloc it once (PATH_MAX) and skip the strlen/malloc/free/repeat
dance, just free it at the end.
+ if (path == NULL)
+ return NULL;
+ n = snprintf(path, len, "%s/.config/swayidle/config",
home);
+ if (n < len && access(path, R_OK) == 0)
+ return path;
+ free(path);
+ } else {
+ len = strlen(config_home) + strlen("/swayidle/config")
+ 1;
+ path = malloc(len);
+ if (path == NULL)
+ return NULL;
+ n = snprintf(path, len, "%s/swayidle/config",
config_home);
+ if (n < len && access(path, R_OK) == 0)
+ return path;
+ free(path);
+ }
+ }
+ len = strlen(SYSCONFDIR "/swayidle/config") + 1;
+ path = malloc(len);
+ if (path == NULL)
+ return NULL;
+ n = snprintf(path, len, "%s/swayidle/config", SYSCONFDIR);
+ if (n < len && access(path, R_OK) == 0)
+ return path;
+ free(path);
I haven't looked closely at parse_line_like_wordexp_does(), but wouldn't
it be simpler to provide an actual replacement function named wordexp()
(i.e. a partial implementation with what's needed by the rest of the code)
so you don't need to touch the code which calls it?
i.e. so you then wouldn't need this block, just keep the original:
+#if HAVE_WORDEXP
wordexp_t p;
wordexp(line, &p, 0);
if (strncmp("timeout", line, i) == 0) {
@@ -1049,6 +1168,24 @@ static int load_config(const char *config_path) {
return -EINVAL;
}
wordfree(&p);
+#else
+ if (strncmp("timeout", line, i) == 0) {
+ int nargs;
+ char **args = parse_line_like_wordexp_does(line,
&nargs);
+ if (args)
+ parse_timeout(nargs, args);
+ else {
+ swayidle_log(LOG_ERROR, "failed parsing \"%s\"
in line %zu", line, lineno);
+ free(line);
+ return -EINVAL;
+ }
+ } else {
+ line[i] = 0;
+ swayidle_log(LOG_ERROR, "Unexpected keyword \"%s\" in
line %zu", line, lineno);
+ free(line);
+ return -EINVAL;
+ }
+#endif
}
free(line);
fclose(f);
