Re: Metafication in error messages (Was: [PATCH] unmetafy Re: $var not expanded in ${x?$var})

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Sat, Feb 24, 2024 at 1:47 AM Stephane Chazelas <stephane@xxxxxxxxxxxx> wrote:
>
> 2024-02-23 14:32:49 -0800, Bart Schaefer:
> [...]
> > More relevant to this discussion is that math errors are one of the
> > two existing callers using the %l format, so any attempt to improve
> > this is going to require changing those calls anyway.
>
> I don't see why we'd need to change the call to zerr in those
> cases. Just fix printf.

That doesn't resolve the issue that math.c:checkunary() is
miscalculating the truncation width even before calling zerr().

Having looked more closely now, if I fix that calculation, then
checkunary() can do the truncation itself and doesn't need to use %l
any longer.

> But in the case of ${var?err}, the err is already metafied, so
> if you make %l take unmetafied input, you're just moving the
> unmetafication to the caller which is counterproductive as it
> makes it break on NULs.

No, it doesn't make it break on NULs, because unmetafy() returns the
length of the resulting string including the NULs, which we can then
pass down to zerr().

The actual tradeoff to make %l do the raw output is:
+ fix the width calculation in math.c, then use %s *
+ add one call to metafy() in parse.c, to use %s instead of %l **
+ add one call to unmetafy() in subst.c, to use %l instead of %s
- remove metalen(), zhalloc(), memcpy(), and nicezputs() from utils.c
+ add one call to fwrite() in place of the above

* Which we should do anyway, so not really a tradeoff
** It's not even certain that's required, given that we're outputting
a string from the lexer

> Also %l is intended (at least in the one case I saw it used) to
> truncase user input, so it should be nicezputs'ed.

Unfortunately it's used *incorrectly* to truncate user input at that one place.

> To me, the only things to do are:
>
> 1. add a %S for raw output (expects metafied input like
>    everything else) to tbe used by ${var[:]?error} and likely only
>    those.

That adds another branch in utils.c with at least the same unmetafy()
+ fwrite(), instead of removing code that the callers no longer need.

> 2. Add missing metafy in bin_print (and possibly elsewhere)
>    before calling the math parser

Needed either way.

> 3. Fix those cases where zerrmsg is called with %s/%l/%S
>    arguments non-metafied like in that "bad interpreter" case
>    above.

That's a whack-a-mole hunt and also orthogonal to whether or not to add %S.

> 4. (optional): Improve %l usages to truncate based on number of
>    characters rather than bytes or at least avoid cutting
>    characters in the middle.

Not needed.