Le Fri, Nov 10, 2023 at 08:44:49AM +0000, Stuart Henderson a écrit :
> 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 :)
the idea behind 'keeping it' is only to be able to upstream it without
too much intrusion, eg
https://github.com/swaywm/swayidle/pull/154 &
https://github.com/swaywm/swaylock/pull/325
> 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.
agreed, might be simple. my C is rusty (and my rust is inexistant..)
> 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:
sure, but that wouldnt fight against wordexp() usage itself ...
not sure that's a hill i'd die on, but at least the discussion with
upstream is started now. Depending on how it goes i'll rework the
patches.
