Re: readall bugs [PATCH]

[arnold]

Hi.

Sorry for the delay in reviewing the patch. It looks pretty good.
Some small notes:

1. In awkgram.y, add a comment that we purposely don't use Node_var_array so
   that rwarray:readall will work.

2. In rwarray.c, I prefer '& var' vs. '&var'; your changes cause
   the code to be inconsistent.

Please merge to gawk-5.2-stable and then merge gawk-5.2-stable
into master.

Thanks!

Arnold

"Andrew J. Schorr" <aschorr@telemetry-investments.com> wrote:

> Hi,
>
> On Wed, Sep 28, 2022 at 08:53:24AM -0400, Andrew J. Schorr wrote:
> > It turns out that readall has some big problems that were not exposed by the
> > existing, inadequate test case. I need to dig in.
>
> I think the attached patch against the master branch fixes the readall
> problems. There were 3 causes of the brain damage:
>
> 1. The gawkapi.c:api_sym_update function was not correctly converting
> untyped variables to arrays. It strangely worked in cases where such variables
> were passed as arguments to functions, but not when the array was accessed
> directly by its global name.
>
> 2. The parser awkgram.y had some inconsistent behavior when instantiating 
> array
> variables. If the first time the variable appeared in the program, it was used
> with a subscript, then the variable was inserted into the global symbol table
> as an array (with type Node_var_array). But if that wasn't the case, for
> example if the first reference to the variable name was as a function call
> argument, then it was created as an untyped variable (with type Node_var_new).
> At execution time, gawk happily converts the untyped variable to an array as
> needed, but this inconsistency in parsing made it impossible for the readall
> function to understand whether the program was using the variable as an array
> already whose contents shouldn't be stomped on. The fix is simply to modify 
> the
> parser to create the variable as untyped, even if the first instance is with a
> subscript.
>
> 3. The extension/rwarray.c code had a hack that attempted to work around
> the issue in #2 above, but it was not a solution that worked in all cases.
> With #2 fixed, there's no need for the hack, and the readall code can
> be simplified and should now be robust.
>
> Please give this a try if you have a chance. I have enhanced the test cases
> to expose some of the things that were broken previously.
>
> It passes "make check" and "make valgrind" both with and without .developing
> on CentOS 7.9.
>
> Arnold -- please let me know if this is OK to commit.
>
> Regards,
> Andy