[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] devel: fix segfault by unset 'assoc[${x[0]}]'

From: Chet Ramey
Subject: Re: [PATCH] devel: fix segfault by unset 'assoc[${x[0]}]'
Date: Wed, 6 Oct 2021 15:20:38 -0400
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:91.0) Gecko/20100101 Thunderbird/91.1.2

On 10/5/21 10:39 PM, Koichi Murase wrote:

>> You're right, there should be no `nesting' considered at all. By the time
>> unbind_array_element is called, since it's only called from unset_builtin,
>> the word and subscript should have already undergone all the expansion
>> they're going to. There should be no need to interpret ${} or $() in the
>> subscript: since associative arrays can have arbitrary subscripts, you
>> should not attempt to parse the subscript contents.
> Yeah, I think the above paragraph describes the expected behavior when
> `assoc_expand_once' is turned on.
> But in this patch, I actually aim to fix the behavior of the
> backward-compatible mode (with `assoc_expand_once' turned off).  In
> the patch, I suggested to remove `(var && assoc_p(var))' from the
> skipsubscript flag for the nesting consideration 

I understand. I'd like the default behavior to be closer to what it is
when assoc_expand_once is enabled, as I said back in March. I think it's
going to be better for users in the long run.

>> It can shortcut and say "ok, if it passes valid_array_reference, we should
>> just consider the entire argument as one word as long as the final
>> character is `]'." This is again where backwards compatibility and
>> assoc_expand_once matter.
>> We can apply your change, but it is still incomplete
> What is exactly the incompleteness that you focus on in this context?

Only that I'd like a more comprehensive behavioral change. Your fix is
fine for the limited scope it tackles (resolving the discrepancy between
valid_array_reference and unbind_array_element).

>> The real issue is that it's still going to expand the subscript for
>> backwards compatibility. I think that's going to have to be solved
>> at a higher level.
> Yes, but I feel like this is another design issue which is irrelevant
> for the fix of the present small problem.

Sure, but that's why we're talking through the issue. Your fix is fine
for the problem it intends to solve, now I'd like to go beyond it and
figure out a better architectural solution.


``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU    chet@case.edu    http://tiswww.cwru.edu/~chet/

reply via email to

[Prev in Thread] Current Thread [Next in Thread]