Another code bogosity

[krader1961]

While working on issue #1038 I came across this block of code:

ast/src/cmd/ksh93/sh/array.c

Lines 1102 to 1108 in cef64df

	int scan = 0;
	if (ap) scan = ap->flags & ARRAY_SCAN;
	if ((mode & NV_ASSIGN) && (cp[1] == '=' || cp[1] == '+')) mode |= NV_ADD;
	nv_putsub(np, sp, 0,
	((mode & NV_ADD) ? ARRAY_ADD : 0) |
	(cp[1] && (mode & NV_ADD) ? ARRAY_FILL : mode & ARRAY_FILL));
	if (scan) ap->flags |= scan;

At first glance it appears the ap->flags |= scan does not need to be predicated on if (scan). After all, scan is initialized to zero and thus merging it into ap->flags does nothing if scan is zero. In actuality the if (scan) should be if (ap). I'm creating this issue to raise awareness that there are too many places in the code with this type of logic error.

[krader1961]

Gah! The issue is that nv_putsub() can invalidate (i.e., free) the structure pointed to by ap. But only if the ARRAY_SCAN bit isn't set. So the code as written is correct if a bit obtuse. This is another manifestation of issue #828. Resizing the actual data array can also cause the structure that manages it to be moved. The sooner #828 is fixed the better.