Re: Fix assigning array variables in ksh
пт, 5 мар. 2021 г. в 18:14, Theo Buehler :
>
> On Sun, Feb 21, 2021 at 10:04:07PM +0300, Vadim Zhukov wrote:
> > Hello all.
> >
> > This continues the 'Another potential ksh bug?' thread on misc@:
> > https://marc.info/?l=openbsd-misc=160736700220621=2
> > This present is a bit too late for Christmas, but at least the Day of
> > Red Army is coming soon. I'm sure noone is against this patch then.
> >
> > The code in typeset() function, which is responsible for almost all
> > shell variable tweaking, contains a bug: in case of array it passes
> > "foo=var" instead of only variable name ("foo"), if the value was
> > ever given. This results in, e.g., a bug reported by Jordan Geoghegan.
> > Generally speaking, we had creating a (temporary) variable via global()
> > in vpbase, and of course it didn't get the read-only marker.
> >
> > This fix is to use 'tvar' variable, which always contains shell
> > variable name, without value.
>
> ok tb
>
> > As a bonus, this fixes the ifs.t:IFS-null-1 test (previously failing).
> > I'm too lazy to check why. :-)
>
> I'm confused. I have 11 failing tests before and after your patch.
> Also IFS-null-1 has been passing for a long time.
Hm-m-m. Probably something in my environment... I'll experiment separately then.
Thanks!
Re: Fix assigning array variables in ksh
On Sun, Feb 21, 2021 at 10:04:07PM +0300, Vadim Zhukov wrote:
> Hello all.
>
> This continues the 'Another potential ksh bug?' thread on misc@:
> https://marc.info/?l=openbsd-misc=160736700220621=2
> This present is a bit too late for Christmas, but at least the Day of
> Red Army is coming soon. I'm sure noone is against this patch then.
>
> The code in typeset() function, which is responsible for almost all
> shell variable tweaking, contains a bug: in case of array it passes
> "foo=var" instead of only variable name ("foo"), if the value was
> ever given. This results in, e.g., a bug reported by Jordan Geoghegan.
> Generally speaking, we had creating a (temporary) variable via global()
> in vpbase, and of course it didn't get the read-only marker.
>
> This fix is to use 'tvar' variable, which always contains shell
> variable name, without value.
ok tb
> As a bonus, this fixes the ifs.t:IFS-null-1 test (previously failing).
> I'm too lazy to check why. :-)
I'm confused. I have 11 failing tests before and after your patch.
Also IFS-null-1 has been passing for a long time.
>
> Okay anyone?
>
> --
> WBR,
> Vadim Zhukov
>
>
> Index: regress/bin/ksh/obsd-regress.t
> ===
> RCS file: /cvs/src/regress/bin/ksh/obsd-regress.t,v
> retrieving revision 1.10
> diff -u -p -r1.10 obsd-regress.t
> --- regress/bin/ksh/obsd-regress.t8 Dec 2018 21:03:51 - 1.10
> +++ regress/bin/ksh/obsd-regress.t21 Feb 2021 18:51:54 -
> @@ -503,3 +503,16 @@ description:
> stdin:
> kill -s SIGINFO $$
> ---
> +
> +name: overwrite-ro-array
> +description:
> + do not allow to override first element of a read-only array
> + via the non-array access.
> +stdin:
> + arr[0]=foo
> + readonly arr
> + arr=bar
> +expected-exit: e == 1
> +expected-stderr-pattern:
> + /: arr: is read only$/
> +---
> Index: bin/ksh/var.c
> ===
> RCS file: /cvs/src/bin/ksh/var.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 var.c
> --- bin/ksh/var.c 21 Feb 2020 18:21:23 - 1.71
> +++ bin/ksh/var.c 21 Feb 2021 18:51:54 -
> @@ -644,7 +644,7 @@ typeset(const char *var, int set, int cl
> global(tvar);
> set &= ~(LOCAL|LOCAL_COPY);
>
> - vpbase = (vp->flag & ARRAY) ? global(arrayname(var)) : vp;
> + vpbase = (vp->flag & ARRAY) ? global(arrayname(tvar)) : vp;
>
> /* only allow export flag to be set. at ksh allows any attribute to
>* be changed, which means it can be truncated or modified
>
Re: Fix assigning array variables in ksh
On 2/21/21 11:04 AM, Vadim Zhukov wrote:
> Hello all.
>
> This continues the 'Another potential ksh bug?' thread on misc@:
> https://marc.info/?l=openbsd-misc=160736700220621=2
> This present is a bit too late for Christmas, but at least the Day of
> Red Army is coming soon. I'm sure noone is against this patch then.
>
> The code in typeset() function, which is responsible for almost all
> shell variable tweaking, contains a bug: in case of array it passes
> "foo=var" instead of only variable name ("foo"), if the value was
> ever given. This results in, e.g., a bug reported by Jordan Geoghegan.
> Generally speaking, we had creating a (temporary) variable via global()
> in vpbase, and of course it didn't get the read-only marker.
>
> This fix is to use 'tvar' variable, which always contains shell
> variable name, without value.
>
> As a bonus, this fixes the ifs.t:IFS-null-1 test (previously failing).
> I'm too lazy to check why. :-)
>
> Okay anyone?
>
> --
> WBR,
> Vadim Zhukov
>
>
> Index: regress/bin/ksh/obsd-regress.t
> ===
> RCS file: /cvs/src/regress/bin/ksh/obsd-regress.t,v
> retrieving revision 1.10
> diff -u -p -r1.10 obsd-regress.t
> --- regress/bin/ksh/obsd-regress.t8 Dec 2018 21:03:51 - 1.10
> +++ regress/bin/ksh/obsd-regress.t21 Feb 2021 18:51:54 -
> @@ -503,3 +503,16 @@ description:
> stdin:
> kill -s SIGINFO $$
> ---
> +
> +name: overwrite-ro-array
> +description:
> + do not allow to override first element of a read-only array
> + via the non-array access.
> +stdin:
> + arr[0]=foo
> + readonly arr
> + arr=bar
> +expected-exit: e == 1
> +expected-stderr-pattern:
> + /: arr: is read only$/
> +---
> Index: bin/ksh/var.c
> ===
> RCS file: /cvs/src/bin/ksh/var.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 var.c
> --- bin/ksh/var.c 21 Feb 2020 18:21:23 - 1.71
> +++ bin/ksh/var.c 21 Feb 2021 18:51:54 -
> @@ -644,7 +644,7 @@ typeset(const char *var, int set, int cl
> global(tvar);
> set &= ~(LOCAL|LOCAL_COPY);
>
> - vpbase = (vp->flag & ARRAY) ? global(arrayname(var)) : vp;
> + vpbase = (vp->flag & ARRAY) ? global(arrayname(tvar)) : vp;
>
> /* only allow export flag to be set. at ksh allows any attribute to
>* be changed, which means it can be truncated or modified
>
Hi Vadim,
I just tried this patch out and it appears to fix the issue -- thanks! I tried
running a few large shell scripts that make use of arrays and everything worked
as expected.
I don't have the authority to give you an ok, but I would certainly like to see
this patch accepted.
Regards,
Jordan
Fix assigning array variables in ksh
Hello all.
This continues the 'Another potential ksh bug?' thread on misc@:
https://marc.info/?l=openbsd-misc=160736700220621=2
This present is a bit too late for Christmas, but at least the Day of
Red Army is coming soon. I'm sure noone is against this patch then.
The code in typeset() function, which is responsible for almost all
shell variable tweaking, contains a bug: in case of array it passes
"foo=var" instead of only variable name ("foo"), if the value was
ever given. This results in, e.g., a bug reported by Jordan Geoghegan.
Generally speaking, we had creating a (temporary) variable via global()
in vpbase, and of course it didn't get the read-only marker.
This fix is to use 'tvar' variable, which always contains shell
variable name, without value.
As a bonus, this fixes the ifs.t:IFS-null-1 test (previously failing).
I'm too lazy to check why. :-)
Okay anyone?
--
WBR,
Vadim Zhukov
Index: regress/bin/ksh/obsd-regress.t
===
RCS file: /cvs/src/regress/bin/ksh/obsd-regress.t,v
retrieving revision 1.10
diff -u -p -r1.10 obsd-regress.t
--- regress/bin/ksh/obsd-regress.t 8 Dec 2018 21:03:51 - 1.10
+++ regress/bin/ksh/obsd-regress.t 21 Feb 2021 18:51:54 -
@@ -503,3 +503,16 @@ description:
stdin:
kill -s SIGINFO $$
---
+
+name: overwrite-ro-array
+description:
+ do not allow to override first element of a read-only array
+ via the non-array access.
+stdin:
+ arr[0]=foo
+ readonly arr
+ arr=bar
+expected-exit: e == 1
+expected-stderr-pattern:
+ /: arr: is read only$/
+---
Index: bin/ksh/var.c
===
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.71
diff -u -p -r1.71 var.c
--- bin/ksh/var.c 21 Feb 2020 18:21:23 - 1.71
+++ bin/ksh/var.c 21 Feb 2021 18:51:54 -
@@ -644,7 +644,7 @@ typeset(const char *var, int set, int cl
global(tvar);
set &= ~(LOCAL|LOCAL_COPY);
- vpbase = (vp->flag & ARRAY) ? global(arrayname(var)) : vp;
+ vpbase = (vp->flag & ARRAY) ? global(arrayname(tvar)) : vp;
/* only allow export flag to be set. at ksh allows any attribute to
* be changed, which means it can be truncated or modified
