On 2023/11/10 10:14, Landry Breuil wrote:
> 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
Oh I would suggest upstreaming that too, surely they don't need wordexp
in that part either?
> > 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.
>
