Re: [PATCH] Refactor resolve_nameref() and define getparam(), loadparama(), and resolveparam()

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On Thu, Sep 10, 2026 at 7:27 PM Philippe Altherr
<philippe.altherr@xxxxxxxxx> wrote:
>
> The main aim of this patch was to enhance resolve_nameref() to also return the last followed reference but it ended up defining a mini-API for getting, loading and resolving parameters. The newly introduced getparam(), loadparam(), and resolveparam() functions have names that explicitly state what they are doing and their behavior is well defined and well documented.
>
> In addition of defining getparam(), loadparam(), and resolveparam() and replacing all calls to resolve_nameref() with calls to resolveparam(), the patch also does the following:

loadparam() may be clearly defined, but at the end of your patch
series, it has no callers in the codebase. Remove? loadparam_pm only
has one caller (excluding the unused one in loadparam), maybe it
should be a static helper instead, since the caller is static? That
absolves you from wanting to keep the unused string name wrapper for
symmetry with get/resolve/resolveref.

> Defines isset_pm() and asset_pm() to test/ensure whether/that a parameter is a set one.

You have getparam/getparam_pm, loadparam/loadparam_pm and
resolveparam(ref)/resolveparam(ref)_pm where the non-_pm are wrappers
that call getparam() on the name for the caller. But then you have
isset_pm and asset_pm with no corresponding isset or asset, and in
fact, isset() is an existing macro that checks if an option is set.
Maybe call these latter two param_isset and param_asset instead, since
the _pm doesn't denote a difference from a version that takes a string
name argument (i'm not sold on "asset" either btw).

- /* Check for self references */
- if (refname && *refname && basepm != pm) {
-    dont_queue_signals(); /* Prevent unkillable loops */
-    basepm = resolve_nameref_rec(pm, pm, 0);
-    restore_queue_signals(q);
- }

Do we have some proof that these unkillable loops can not happen
anymore? I couldn't find some motivation for removing this.

Other than that I haven't spotted anything wrong (that includes all
the patches in the series leading up to your 'eliminate-paramtab'
branch head), and less dependence on dancing around swapping global
pointers is probably a good goal.

-- 
Mikael Magnusson