Re: [PATCH] Simplify alias storage.

On 06/04/2024 08:19, Herbert Xu wrote:
> On Wed, Jan 11, 2023 at 06:23:41PM +0000, Harald van Dijk wrote:
>>
>>   	if (ap) {
>>   		if (!(ap->flag & ALIASINUSE)) {
>> -			ckfree(ap->val);
>> +			ckfree(ap->name);
>>   		}
> 
> I think this breaks ALIASINUSE.  When an alias is marked as INUSE,
> it will be freed by input.c (popstring).  However, your patch doesn't
> touch input.c at all.

You're quite right, thanks for spotting that. A test case that covers 
that is

   alias foo='
   alias foo="echo Hello"
   '
   foo
   foo

Here, the storage of the original 'foo' alias needs to be kept while the 
expansion is ongoing, even when the second 'alias' overwrites the 
original definition. This bug is fixed by below additional patch.

However, the patch conflicts with c8db655b because scanning 'name' will 
now find the '=' character and continue on from that to check the value. 
That should be relatively easy to avoid, but I am not sure it is worth it.

Cheers,
Harald van Dijk

diff --git a/src/input.c b/src/input.c
index 38969a7..4604945 100644
--- a/src/input.c
+++ b/src/input.c
@@ -386,7 +386,7 @@ pushstring(char *s, void *ap)
  	sp->ap = (struct alias *)ap;
  	if (ap) {
  		((struct alias *)ap)->flag |= ALIASINUSE;
-		sp->string = s;
+		sp->string = ((struct alias *)ap)->name;
  	}
  	parsefile->nextc = s;
  	parsefile->nleft = len;
@@ -405,7 +405,7 @@ static void popstring(void)
  		    parsefile->nextc[-1] == '\t') {
  			checkkwd |= CHKALIAS;
  		}
-		if (sp->string != sp->ap->val) {
+		if (sp->string != sp->ap->name) {
  			ckfree(sp->string);
  		}
  	}