Skip to content

compond_variable assignment #889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

kalpitha-ibm
Copy link

@kalpitha-ibm kalpitha-ibm commented Jul 7, 2025

Adding fix on behalf on IBM AIX
This is related assignment of compound_variables.

Below is the example test case to assign compond_variable
using set-x to display the values.

set -x

complex=( real=1 img=2 )

  • complex.real=1
  • complex.img=2

complex=( real1=3 img1=4 )

  • complex.real1=3
  • complex.img1=4

RC=$?

  • RC=0

real=echo "${complex.real}"

  • echo ''
  • real='' >> here the variable should be 1

img=echo "${complex.img}"

  • echo ''
  • img='' >> here the variable should be 2

real1=echo "${complex.real1}"

  • echo 3
  • real1=3

img1=echo "${complex.img1}"

  • echo 4
  • img1=4

The goal of the test was to ensure all parts of the complex compound variable were preserved after setting additional fields. The fix ensures that the variable isn't overwritten and that all expected values (1, 2, 3, 4) are correctly retained and verified.

 compond_variable is not assigned properly
@JohnoKing
Copy link

I was able to reproduce the bug on all current ksh93 releases. It appears to be a regression introduced in ksh93u- 2010-08-11. This PR has some problems though:

- The PR doesn't compile because of 93u+m's changes to np->nvalue (was the patch was originally written against 2012-08-01 93u+?). Compiler error output:

/home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:515:26: error: member reference base type 'void *' is not a structure or union
  515 |                                         else if(((np->nvalue.cp && np->nvalue.cp!=Empty)) && !nv_type(np))
      |                                                   ~~~~~~~~~~^~~
/home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:515:43: error: member reference base type 'void *' is not a structure or union
  515 |                                         else if(((np->nvalue.cp && np->nvalue.cp!=Empty)) && !nv_type(np))
      |                                                                    ~~~~~~~~~~^~~

- The PR does not include a regression test.

These problems can be fixed with a patch:

diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c
index b35624a62..52185d717 100644
--- a/src/cmd/ksh93/sh/name.c
+++ b/src/cmd/ksh93/sh/name.c
@@ -512,7 +512,7 @@ void nv_setlist(struct argnod *arg,int flags, Namval_t *typ)
 						if(!nv_isnull(np) && np->nvalue!=Empty && !nv_isvtree(np))
 							sub=1;
 					}
-					else if(((np->nvalue && np->nvalue!=Empty)||nv_isvtree(np)|| nv_arrayptr(np)) && !nv_type(np))
+					else if(np->nvalue && np->nvalue!=Empty && !nv_type(np))
 					{
 						int was_assoc_array = ap && ap->fun;
 						nv_unset(np,NV_EXPORT);  /* this can free ap */
diff --git a/src/cmd/ksh93/tests/comvar.sh b/src/cmd/ksh93/tests/comvar.sh
index 27bd77f4b..2d5aca417 100755
--- a/src/cmd/ksh93/tests/comvar.sh
+++ b/src/cmd/ksh93/tests/comvar.sh
@@ -733,5 +733,22 @@ exp=$'(\n\ttypeset -C -a c\n)'
 [[ $got == "$exp" ]] || err_exit 'setting compound array c.c=() does not preserve -C attribute' \
 	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 
+# ======
+# github.com/ksh93/ksh/pull/889
+unset complex real img img1 real1
+complex=( real=1 img=2 )
+complex=( real1=3 img1=4 )
+real=$(echo "${complex.real}")
+img=$(echo "${complex.img}")
+real1=$(echo "${complex.real1}")
+img1=$(echo "${complex.img1}")
+exp='real=1
+img=2
+real1=3
+img1=4'
+got=$(typeset -p real img real1 img1)
+[[ $exp == "$got" ]] || err_exit 'previous fields of complex compound variable are lost when adding additional fields' \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))

After patching the pull request to make it compile, another problem is evident; the bugfix causes some regression test failures:

# Run the regression tests with './bin/shtests' at the root of the repository
test arrays begins at 2025-07-07+14:55:56
	arrays.sh[606]: FAIL: setting element 1 of array to compound variable failed (expected 'typeset -a foo=((11 22) (x=3))', got 'typeset -a foo=((11 22) (44 55) )')
test arrays failed at 2025-07-07+14:55:56 with exit code 1 [ 186 tests 1 error ]
test comvar begins at 2025-07-07+14:56:03
	comvar.sh[659]: FAIL: assignment of compound variable to compound array element not working
test comvar failed at 2025-07-07+14:56:03 with exit code 1 [ 105 tests 1 error ]
test subshell begins at 2025-07-07+14:56:43
	subshell.sh[100]: FAIL: compound variable changes after unset leaves
test subshell failed at 2025-07-07+14:56:54 with exit code 1 [ 154 tests 1 error ]
Total errors: 3

The regression this PR attempts to fix was introduced via the following vague and nondescript patch to name.c in 93u- 2010-08-11:

--- a/src/cmd/ksh93/sh/name.c
+++ b/src/cmd/ksh93/sh/name.c
@@ -488,7 +488,7 @@ void nv_setlist(register struct argnod *arg,register int flags, Namval_t *typ)
 						if(!nv_isnull(np) && np->nvalue.cp!=Empty && !nv_isvtree(np))
 							sub=1;
 					}
-					else if(np->nvalue.cp && np->nvalue.cp!=Empty && !nv_type(np))
+					else if(((np->nvalue.cp && np->nvalue.cp!=Empty)||nv_isvtree(np)) && !nv_type(np))
 						_nv_unset(np,NV_EXPORT);
 				}
 				else

This pull request blindly reverts the 2010-08-11 change without trying to ascertain the code's intent or purpose, and consequently introduces regressions that manifest as three regression test failures. The bugfix will need some work.

@kalpitha-ibm
Copy link
Author

Thanks for your comments and suggestions!
I will check on these lines and regression caused by the fix and try to come up with wholesome fix for the issue.
Yes, The initial fix originally was written against 2012-08-01 93u+ code on AIX IBM. So now we will come up with fix for the latest community code.

@kalpitha-ibm
Copy link
Author

@JohnoKing Hello !!
Need your help on running test suite complete bucket on AIX.
On AIX we are not able to run the entire test bucket at once just like you have run in the above comment.
We are running script by script replacing err_exit with echo which is consuming more time for testing. Is there any possible suggestion you can provide us with how to run the complete test bucket at once on AIX.
Thanks in Advance!!!

@JohnoKing
Copy link

You can use an older version of the shtests wrapper script circa f811482: https://github.com/ksh93/ksh/blob/f8114823/bin/shtests

Copy it to the bin folder of the repository, build ksh, then run it with bin/shtests (you also really shouldn't replace err_exit with echo). For ancient 93u+ you should define the KSH variable manually to avoid bugs in the bin/package script.

So, e.g.:

$ cd path/to/ksh93u+m
$ git checkout f8114823 -- bin/shtests
$ cd path/to/93u+
$ cp path/to/ksh93u+m/bin/shtests ./bin
$ KSH=$(echo $PWD/arch/*/bin/ksh) bin/shtests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants