On Fri, Aug 04, 2023 at 09:44:40PM +0200, Michiel van den Heuvel wrote:
This patch adds support for environment variable expansion to the
configuration options that encode a path. This would get rid of some
templating for my own configuration, and might prove useful for others
with integrating with notmuch, or having the same configuration file
for multiple very different systems (my usecase).
related discussion: https://sourceforge.net/p/isync/feature-requests/17/
- Bash - or perhaps more correct - Makefile syntax is used. To keep
the implementation simple, a double dollar sign is used to represent
a literal, instead of a backslash.
i can see why you would do that, but it's getting a bit messy, as
technically you'd need to use (only) $() for make-like variable
expansion. also, the tilde expansion is a bash thing. one might get
confused ...
considering the discussion on the tracker, it's worth considering doing
the expansion at the parser level already. that would also enable
backslash-escaping.
but then, there would be a problem with *Cmd due to suddenly requiring
escaping of $. make-like layering of the expansions on top actually
doesn't seem too bad, though it's imperfect, as $() is valid bash/posix
syntax as well. hmm ...
- I'd normally use valgrind to check for off-by-one and out of bounds
errors, but it didn't work with mbsync on my system.
weird. i didn't run it for quite a while, but previously vg worked just
fine on isync.
two comments on the patch from a cursory read:
+ val = getenv( nfstrndup( s, e - s ) );
this is leaking.
+ r += nfsnprintf( r, sizeof(path) - (r - path), "%s",
val );
this may fail due to an unexpectedly large variable content, which is
sort of user-controlled. that makes oob() kinda inappropriate (which is
supposed to catch internal errors).
regards
_______________________________________________
isync-devel mailing list
isync-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/isync-devel
