Re: [PATCH v2 3/5] setup: add 'allow_dot' arg to path_allowlist_apply()

On Fri, Aug 14, 2026 at 8:12 PM Junio C Hamano [off-list ref] wrote:

If this is just "I want to add an extra caller that has specific
need and do not care about others in the future", this may be OK but
as a public function, this is a bit disappointing API design.

I thought that flags might be enough at least for some time, but I
agree that it could soon make the code difficult to reason about,
which is not a good thing for this kind of code.

I expected, as a generally useful function, you would instead add a
callback function to allow replacing the use of is_absoute_path()
plus the warning there, i.e.

void path_allowlist_apply(const char *key, const char *value,
                          const char *target_path, bool *matches,
                          bool (*allow_path)(const char *path))
{
        ...

        if (!allow_path(allowed))
                goto end;

Also to avoid limiting this to configuration callback, I might
recommend to have it be more like this:

void path_allowlist_apply(const char *allowed, const char *target_path,
                          bool *matches,
                          bool (*allow_path)(const char *path, void *cbdata),
                          void *allow_path_cbdata)

where the original safe-directory thing may call
git_config_pathname() to compute allowed before calling this helper,
and pass the address of something like:

        struct { const char *key, *value } cbdata = {
                .key = key, .value = value;
        };

as the cbdata, and pass something like this

        static bool allow_safe_dir(const char *path, void *cbdata_)
        {
                struct { const char *key, *value } *cbdata = _cbdata;
                if (is_absoute_path(path) || !strcmp(path, ".")
                        return true; /* ok */

                warning(_("%s '%s' not absolute"), cbdata->key, path);
                return false;
        }

as the allow_path callback function.  IOW warning, or insisting on
it being absolute, etc., does not have to be carved in stone.

I have tried to implement it like you suggest in the v3 I just sent.

Thanks.