Re: param expansion with single-character special vars in the environment
On 28 Apr 2016, at 02:36, Grisha Levit wrote: > > On Wed, Apr 27, 2016 at 6:47 PM, Piotr Grzybowski wrote: > + if (v == 0 && onref || ( onref && !legal_identifier(value))) > > Shouldn't this (and the other patch) check valid_array_reference as well to > support declare -n ref=a[x] ? yes most certainly should, and also should do a bit more. frankly, your original report: readonly TMOUT=5; ref=$TMOUT; declare -n ref; ref=10; declare -n TMOUT; read really scares me. The fix should also go to the nameref code, since I refuse to sanction the apparent entry in the variable table with name="5" and value="10" after this code is run, just by the fact that $TMOUT value is not a valid identifier. cheers, pg
Re: param expansion with single-character special vars in the environment
On Wed, Apr 27, 2016 at 6:47 PM, Piotr Grzybowski wrote: > + if (v == 0 && onref || ( onref && !legal_identifier(value))) > Shouldn't this (and the other patch) check valid_array_reference as well to support declare -n ref=a[x] ?
Re: declare [-+]n behavior on existing (chained) namerefs
> interpret "on the variable" as doing a depth first search (going as deep
as possible over the reference chain) in order to find the variable
Yes, that makes perfect sense, but I think all the issues above don't
concern the search for the variable, but rather modifications to the
references themselves.
This might just be a documentation issue, as the docs state that something
different should be happening:
1. It seems unambiguous from the description that `declare +n ref_1' should
remove the nameref attribute from ref_1, not from any other variable
2. A strict interpretation would also suggest that `declare -n ref_1=foo'
should change the value of the variable referenced by the nameref (or the
chain of references), not the value of ref_N. I don't know if that's
really the implementation's intention -- perhaps adding a note to the "If a
variable name is followed by =value, the value of the variable ..." section
of the declare builtin's description would help clarify this case.
On Wed, Apr 27, 2016 at 5:54 PM, Piotr Grzybowski
wrote:
>
> this one does not seem like a bug to me, rather a decision made by the
> author: to interpret "on the variable" from this:
>
> "All references, assignments, and attribute modifications to name, except
> for changing the -n attribute itself, are performed on the variable
> referenced by name’s value."
>
> as doing a depth first search (going as deep as possible over the
> reference chain) in order to find the variable. The last case you mention
> is discussable: when the leaf is an empty reference should +n do something?
> Probably should do the same as declare +n ref_N; which I think is just to
> leave a variable with identifier ref_N. But then, ref_((N-1)) points to a
> variable and suddenly there is a potentially unexpected change in your
> lovely tree of empty references.
> One can argue: if you pass the ref_i to some function, and you want to
> modify ref_N how to do it? The choice made seems fairy logical, I am not
> saying that it is the best possible one though.
>
> cheers,
> pg
>
>
>
> On 27 Apr 2016, at 21:26, Grisha Levit wrote:
>
> > declare -n name=value, when name is already a nameref, shows the
> following presumably inconsistent behavior:
> >
> > Given a chain of namerefs like:
> >
> > ref_1 -> ref_2 -> ... -> ref_i ... -> ref_N [-> var]
> >
> > • If ref_N points to a name that is not a nameref, the operations
> declare -n ref_N=value and declare +n ref_N modify the value/attributes of
> ref_N (this seems to be the desired behavior)
> > • For i value/attributes of ref_N and not of ref_i
> > • If ref_N is declared as a nameref but unset, then these
> operations on ref_i have no effect.
> > For example, starting with:
> >
> > unset -n ref{1..3
> > }
> >
> > declare
> > -n ref1=ref2 ref2=ref3 ref3=var1
> >
> > # declare -n ref1=var2
> > # declare -p ref{1..4}
> > declare -n ref1="ref2" # unchanged
> > declare -n ref2="ref3"
> > declare -n ref3="var2" # changed
> > # declare +n ref1
> > # declare -p ref{1..3}
> > declare -n ref1="ref2" # unchanged
> > declare -n ref2="ref3"
> > declare -- ref3="var1" # changed, no loner nameref
> > Or alternatively:
> >
> > unset -n ref{1..3
> > }
> >
> > declare
> > -n ref1=ref2 ref2=ref3 ref3
> >
> > # declare +n ref1
> > # declare -p ref{1..3}
> > declare -n ref1="ref2" # unchanged
> > declare -n ref2="ref3" # unchanged
> > declare -n ref3 # unchanged
> > # declare -n ref1=var1
> > # declare -p ref{1..3}
> > declare -n ref1="ref2"
> > declare -n ref2="ref3"
> > declare
> > -n ref3
> >
> > The man page says:
> >
> > All references, assignments, and attribute modifications to name, except
> for changing the -n attribute itself, are performed on the variable
> referenced by name’s value.
> >
> > This does not appear to be the case, as declare -n ref_N=value changes
> $ref_N, not $value, and declare +n ref_i changes ref_N.
> >
>
>
Re: param expansion with single-character special vars in the environment
sorry for the spam concerning the patch: it needs merging with what Eduardo
posted (excluding the `declare -n r; declare -n r;' regression bug), and what
is in #ifdef 0 in declare.def; I just found out that it is exactly what it
intends to do.
I am sure we need to forbid the illegal identifiers during declare -n and in
later assignments to the nameref variable.
cheers,
pg
On 28 Apr 2016, at 00:47, Piotr Grzybowski wrote:
>
> a short summary:
>
> 1. there is a real bug reported here: allowing namref on illegal identifiers,
> if we assume that the value of the variable that has nameref attribute is the
> name of the variable it refers to, then we have to require that declare -n
> a=b anly allows legal identifiers for b:
>
> diff --git a/builtins/declare.def b/builtins/declare.def
> index a1e9b4e..53b688c 100644
> --- a/builtins/declare.def
> +++ b/builtins/declare.def
> @@ -719,7 +719,7 @@ declare_internal (list, local_var)
> if (onref)
>aflags |= ASS_NAMEREF;
> v = bind_variable_value (var, value, aflags);
> - if (v == 0 && onref)
> + if (v == 0 && onref || ( onref && !legal_identifier(value)))
>{
> sh_invalidid (value);
> assign_error++;
>
> 2. the other matter that you, Grisha, reported, and Eduardo commented heavily
> on, is not a matter of creating a nameref, but a matter of overriding
> readonly variables identifiers, a matter of discussion if it should be
> allowed (coproc bugs and getopt discussed lately being other examples). But
> it is not a bug, the decision was made to allow namerefs, they work as
> documented, the readonly variables are not touched, they identifiers are used
> as per definition of nameref. A matter of discussion should be: how to
> protect some variables, like the ones that were mentioned (PATH, TMOUT, etc).
> If the readonly means also "do not give and attributes" then declare -n (and
> others) should be forbidden on readonly variables. On the other hand we can
> think of declaring variable as: `protected a' which would mean that no
> modification of attributes is possible, but variable data can be changed, as
> opposed to `readonly a' which could mean "data of the variable cannot be
> changed" and finally both as, e.g.: `exclusive a' meaning readonly|protected.
>
> 3. I have no idea about this global/local thing Grisha reported.
>
> cheers,
> pg
>
>
>
> On 25 Apr 2016, at 22:57, Grisha Levit wrote:
>
>> A related issue is with adding the nameref attribute to a readonly variable.
>> Not sure if that should be allowed, as it can be used to change the meaning
>> of the variable, even for variables that bash is using internally:
>>
>> $ TIMEFORMAT='%R'
>>
>> $ time bash -c
>> 'readonly TMOUT=5; read'
>> 5.007
>>
>> $ time bash -c
>> 'readonly TMOUT=5; ref=$TMOUT; declare -n ref; ref=10; declare -n TMOUT;
>> read'
>> 10.004
>> The same technique works for circumventing PATH in a restricted shell:
>>
>> $ PATH='/restricted/bin' $BASH -r -c 'type sed; ref=$PATH; declare -n ref;
>> ref=/usr/bin; declare -n PATH; type sed'
>> bash
>> bash: line
>> 0: type
>> : sed: not found
>> sed is /usr/bin/sed
>>
>
Re: param expansion with single-character special vars in the environment
a short summary:
1. there is a real bug reported here: allowing namref on illegal identifiers,
if we assume that the value of the variable that has nameref attribute is the
name of the variable it refers to, then we have to require that declare -n a=b
anly allows legal identifiers for b:
diff --git a/builtins/declare.def b/builtins/declare.def
index a1e9b4e..53b688c 100644
--- a/builtins/declare.def
+++ b/builtins/declare.def
@@ -719,7 +719,7 @@ declare_internal (list, local_var)
if (onref)
aflags |= ASS_NAMEREF;
v = bind_variable_value (var, value, aflags);
- if (v == 0 && onref)
+ if (v == 0 && onref || ( onref && !legal_identifier(value)))
{
sh_invalidid (value);
assign_error++;
2. the other matter that you, Grisha, reported, and Eduardo commented heavily
on, is not a matter of creating a nameref, but a matter of overriding readonly
variables identifiers, a matter of discussion if it should be allowed (coproc
bugs and getopt discussed lately being other examples). But it is not a bug,
the decision was made to allow namerefs, they work as documented, the readonly
variables are not touched, they identifiers are used as per definition of
nameref. A matter of discussion should be: how to protect some variables, like
the ones that were mentioned (PATH, TMOUT, etc). If the readonly means also "do
not give and attributes" then declare -n (and others) should be forbidden on
readonly variables. On the other hand we can think of declaring variable as:
`protected a' which would mean that no modification of attributes is possible,
but variable data can be changed, as opposed to `readonly a' which could mean
"data of the variable cannot be changed" and finally both as, e.g.: `exclusive
a' meaning readonly|protected.
3. I have no idea about this global/local thing Grisha reported.
cheers,
pg
On 25 Apr 2016, at 22:57, Grisha Levit wrote:
> A related issue is with adding the nameref attribute to a readonly variable.
> Not sure if that should be allowed, as it can be used to change the meaning
> of the variable, even for variables that bash is using internally:
>
> $ TIMEFORMAT='%R'
>
> $ time bash -c
> 'readonly TMOUT=5; read'
> 5.007
>
> $ time bash -c
> 'readonly TMOUT=5; ref=$TMOUT; declare -n ref; ref=10; declare -n TMOUT; read'
> 10.004
> The same technique works for circumventing PATH in a restricted shell:
>
> $ PATH='/restricted/bin' $BASH -r -c 'type sed; ref=$PATH; declare -n ref;
> ref=/usr/bin; declare -n PATH; type sed'
> bash
> bash: line
> 0: type
> : sed: not found
> sed is /usr/bin/sed
>
Re: declare [-+]n behavior on existing (chained) namerefs
this one does not seem like a bug to me, rather a decision made by the author:
to interpret "on the variable" from this:
"All references, assignments, and attribute modifications to name, except for
changing the -n attribute itself, are performed on the variable referenced by
name’s value."
as doing a depth first search (going as deep as possible over the reference
chain) in order to find the variable. The last case you mention is discussable:
when the leaf is an empty reference should +n do something? Probably should do
the same as declare +n ref_N; which I think is just to leave a variable with
identifier ref_N. But then, ref_((N-1)) points to a variable and suddenly there
is a potentially unexpected change in your lovely tree of empty references.
One can argue: if you pass the ref_i to some function, and you want to modify
ref_N how to do it? The choice made seems fairy logical, I am not saying that
it is the best possible one though.
cheers,
pg
On 27 Apr 2016, at 21:26, Grisha Levit wrote:
> declare -n name=value, when name is already a nameref, shows the following
> presumably inconsistent behavior:
>
> Given a chain of namerefs like:
>
> ref_1 -> ref_2 -> ... -> ref_i ... -> ref_N [-> var]
>
> • If ref_N points to a name that is not a nameref, the operations
> declare -n ref_N=value and declare +n ref_N modify the value/attributes of
> ref_N (this seems to be the desired behavior)
> • For i value/attributes of ref_N and not of ref_i
> • If ref_N is declared as a nameref but unset, then these operations on
> ref_i have no effect.
> For example, starting with:
>
> unset -n ref{1..3
> }
>
> declare
> -n ref1=ref2 ref2=ref3 ref3=var1
>
> # declare -n ref1=var2
> # declare -p ref{1..4}
> declare -n ref1="ref2" # unchanged
> declare -n ref2="ref3"
> declare -n ref3="var2" # changed
> # declare +n ref1
> # declare -p ref{1..3}
> declare -n ref1="ref2" # unchanged
> declare -n ref2="ref3"
> declare -- ref3="var1" # changed, no loner nameref
> Or alternatively:
>
> unset -n ref{1..3
> }
>
> declare
> -n ref1=ref2 ref2=ref3 ref3
>
> # declare +n ref1
> # declare -p ref{1..3}
> declare -n ref1="ref2" # unchanged
> declare -n ref2="ref3" # unchanged
> declare -n ref3 # unchanged
> # declare -n ref1=var1
> # declare -p ref{1..3}
> declare -n ref1="ref2"
> declare -n ref2="ref3"
> declare
> -n ref3
>
> The man page says:
>
> All references, assignments, and attribute modifications to name, except for
> changing the -n attribute itself, are performed on the variable referenced by
> name’s value.
>
> This does not appear to be the case, as declare -n ref_N=value changes
> $ref_N, not $value, and declare +n ref_i changes ref_N.
>
Re: param expansion with single-character special vars in the environment
On 27 Apr 2016, at 21:49, Eduardo A. Bustamante López wrote: > > Actually, this seems to be a special case, just because '5' is an invalid > variable name. But the problem still exists: > > | dualbus@hp ...src/gnu/bash % ./bash -c 'declare -r RO=x; r=$RO; declare -n > r; x=y; declare -n RO; RO=z; declare -p RO; echo "$RO"' > | declare -nr RO="x" > | z ok lets take a look: > declare -r RO=x; r=$RO; a read only variable RO and variable r with value "x" > declare -n r; from now on every assignment to r assigns to variable of name x > x=y; variavle of name x has value y > declare -n RO; here you make RO a nameref, from now on assignments to RO are not assignments to readonly variable RO but to variable of name "$RO" in this case variable x > RO=z; assigment to variable x via a nameref called RO > declare -p RO; echo "$RO" print the value of the variable referenced to by $RO, in that case x. No assignment to readonly variables whatsoever. if you want to make a nameref that has the same name as the readonly variable, feel free. The only bug I see, is the apparent impossibility of removing the nameref from readonly variable, I assure you, if you could remove the nameref from RO you would recover "x" because it is still there in the entry of variables table, as seen by declare -p. I do not see other bug here. What do you think? cheers, pg
Re: param expansion with single-character special vars in the environment
wait, this one is all right, look: 1. you are making a readonly variable: readonly USER=sandbox 2. then you are creating a nameref: declare -n USER; from now on the assignments to USER are assignments to variable of name sandbox 3. then you create a variable sandbox and assign a value since sandbox variable is not read only, then assignments are allowed. if you do readonly sandbox=someuser you will get errors all around. USER variable value is untouched (looking at the code); the find_varibale calls return the sandbox entry, as they should. pg On 27 Apr 2016, at 14:41, Grisha Levit wrote: > On Wed, Apr 27, 2016 at 7:37 AM, Piotr Grzybowski wrote: > > > > It seems to me that creating the reference should be allowed, but the access > to the referenced variable should honor its attributes. > > > Once you convert the variable to a reference, you can control its value by > modifying the value of a different variable with the name that corresponds to > the value of the readonly variable, so “access to the referenced variable > should honor its attributes” probably won’t do much (If I understand your > suggestion correctly). > > In a less convoluted example that doesn’t rely on creating our own namerefs: > > readonly > USER=sandbox > > USER=root > # bash: USER: readonly variable > > > > declare > -n USER > sandbox=root > # works > > USER=root > # works > > > [[ > $USER == root ]] # 0 > > > > # USER is unchanged, other than the -n attribute > declare -p USER # declare -nrx USER="sandbox" > The above works when the readonly variable has a value that is also a valid > identifier. In my previous example I worked around this using the fact that > ref=; declare -n ref does not check to make sure that $ref is a > valid identifier. > > So: > > readonly > PATH=/opt/bin > > ref= > $PATH > declare > -n ref > ref=/usr/bin > > declare -p /opt/bin # declare -- /opt/bin="/usr/bin" > > > > declare -n PATH # declare -nr PATH="/opt/bin" > echo $PATH # /usr/bin
Re: param expansion with single-character special vars in the environment
On Wed, Apr 27, 2016 at 3:49 PM, Eduardo A. Bustamante López <
dual...@gmail.com> wrote:
> f() { echo $r; }; declare -n r; r=/ f
I'm not sure about the tempenv variable.
>
That case doesn't seem to be an issue since assignments here just create
regular variables without attributes.
$ a=(1); f() { declare -p a; }; a=(2) f
declare -x a="(2)"
Re: param expansion with single-character special vars in the environment
Here's the updated list of cases:
| r=/; declare -n r
| declare -n r=/
| declare -n r; r=/
| declare -n r; for r in /; do :; done
| declare -n r; select r in /; do :; done <<< 1; echo x; echo $r
| declare -n r; ((r=0))
| ((r=0)); declare -n r
| r=/ declare -n r
| f() { declare -n r; }; r=/ f
| f() { echo $r; }; declare -n r; r=/ f
| declare -n r; : ${r:=/}
| declare -n r; exec {r}>/dev/null
| declare -n r; coproc r { :; }; echo $r
| declare -r RO=x; r=$RO; declare -n r; x=y; declare -n RO; RO=z; declare -p
RO; echo "$RO"
| s=/; declare -n r=s; declare -n s; echo $r
| declare -n r=s; declare -n s; s=/
| declare -n r; getopts x r -h
| declare -n r; mapfile r < /dev/null
| mapfile r < /dev/null; declare -n r
| declare -n r; printf -v r /
You can set a nameref value to an invalid identifier, but what I noticed is
that it will write the error message on *expansion* and not on assignment (this
is what bash already does).
The 'mapfile' one is interesting, because it drops the nameref attribute from
the 'r' variable.
--
Eduardo Bustamante
https://dualbus.me/
Re: param expansion with single-character special vars in the environment
On Wed, Apr 27, 2016 at 2:43 PM, Eduardo A. Bustamante López < dual...@gmail.com> wrote: > The attached patch seems to take care of at least these two cases: > I don't think legal_identifier is the correct test here since references to array subscripts or array[@] are valid.
Re: param expansion with single-character special vars in the environment
On Wed, Apr 27, 2016 at 08:58:47PM +0200, Piotr Grzybowski wrote:
> On 27 Apr 2016, at 20:43, Eduardo A. Bustamante López wrote:
>
> > The attached patch seems to take care of at least these two cases: [..]
>
> your patch also adresses the original Grisha's report:
>
> declare -r T=5; ref=$T; declare -n ref; ref=10; declare -n T;
>
> cheers,
> pg
>
>
Actually, this seems to be a special case, just because '5' is an invalid
variable name. But the problem still exists:
| dualbus@hp ...src/gnu/bash % ./bash -c 'declare -r RO=x; r=$RO; declare -n
r; x=y; declare -n RO; RO=z; declare -p RO; echo "$RO"'
| declare -nr RO="x"
| z
The problem here is that it shouldn't be possible to set the nameref attribute
on a variable that already has the readonly attribute set.
The RO variable is still readonly, but you can modify the value it expands to,
by doing that trick.
I compiled the following list of test cases:
| r=/; declare -n r
| declare -n r=/
| declare -n r; r=/
| declare -n r; for r in /; do :; done
| declare -n r; select r in 1; do :; done <<< /
| declare -n r; ((r=0))
| ((r=0)); declare -n r
| r=/ declare -n r
| f() { declare -n r; }; r=/ f
| f() { echo $r; }; declare -n r; r=/ f
| declare -n r; : ${r:=/}
| declare -n r; exec {r}>/dev/null
| declare -n r; coproc r { :; }; echo $r
| declare -r RO=x; r=$RO; declare -n r; x=y; declare -n RO; RO=z; declare -p
RO; echo "$RO"
And my patch seems to address just a few:
| dualbus@hp ...src/gnu/bash % while read -r case; do echo "case: $case";
./bash -c "$case"; done <../cases
| case: r=/; declare -n r
| ./bash: line 0: declare: /: invalid variable name for name reference
| case: declare -n r=/
| ./bash: line 0: declare: /: invalid variable name for name reference
| case: declare -n r; r=/
| ./bash: `/': not a valid identifier
| case: declare -n r; for r in /; do :; done
| ./bash: `/': not a valid identifier
| case: declare -n r; select r in 1; do :; done <<< /
| 1) 1
| #? ./bash: `': not a valid identifier
| case: declare -n r; ((r=0))
| ./bash: `0': not a valid identifier
| case: ((r=0)); declare -n r
| ./bash: line 0: declare: 0: invalid variable name for name reference
| case: r=/ declare -n r
| ./bash: line 0: declare: /: invalid variable name for name reference
| case: f() { declare -n r; }; r=/ f
| environment: line 0: declare: /: invalid variable name for name reference
| case: f() { echo $r; }; declare -n r; r=/ f
| /
| case: declare -n r; : ${r:=/}
| ./bash: `/': not a valid identifier
| case: declare -n r; exec {r}>/dev/null
| ./bash: `10': not a valid identifier
| case: declare -n r; coproc r { :; }; echo $r
| 63
| case: declare -r RO=x; r=$RO; declare -n r; x=y; declare -n RO; RO=z;
declare -p RO; echo "$RO"
| declare -nr RO="x"
| z
The select case seems to be wrong, but I'm too lazy to figure out how to write
it properly. I think the coproc case was recently reported too. I'm not sure
about the tempenv variable.
--
Eduardo Bustamante
https://dualbus.me/
Re: param expansion with single-character special vars in the environment
On 27 Apr 2016, at 20:43, Eduardo A. Bustamante López wrote: > The attached patch seems to take care of at least these two cases: [..] > But I think Chet already considered this, given that the check in declare.def > was '#if 0'ed out. maybe for a reason, after your patch: bash-4.4$ declare -n r bash-4.4$ declare -n r bash: `(null)': not a valid identifier followed by exit(1) It seems to me that this whole nameref business is a bit messy. pg
Re: param expansion with single-character special vars in the environment
On Wed, Apr 27, 2016 at 3:10 PM, Eduardo A. Bustamante López <
dual...@gmail.com> wrote:
> On Mon, Apr 25, 2016 at 03:48:17PM -0400, Grisha Levit wrote:
> > # Expected behavior:set -- 1 2 3echo ${@@A} # set -- '1' '2'
>
> This is actually expected behaviour.
Sorry, that was confusing. I included the expected (and observed) behavior
for comparison to the unexpected behavior that followed. That is, after
the declare/ref= statements, the expected behavior was no longer observed.
declare [-+]n behavior on existing (chained) namerefs
declare -n name=value, when name is already a nameref, shows the following presumably inconsistent behavior: Given a chain of namerefs like: ref_1 -> ref_2 -> ... -> ref_i ... -> ref_N [-> var] - If ref_N points to a name that is not a nameref, the operations declare -n ref_N=value and declare +n ref_N modify the value/attributes of ref_N (this seems to be the desired behavior) - For i
Re: param expansion with single-character special vars in the environment
On Mon, Apr 25, 2016 at 03:48:17PM -0400, Grisha Levit wrote:
> There seems to be a bug that if an array variable with a name matching one
> of the special single-character variables exists, then that variable is
> used during substring expansion and parameter transformation.
[...]
> # Expected behavior:set -- 1 2 3echo ${@@A} # set -- '1' '2'
> '3'echo ${@:0} # bash 1 2 3
[...]
This is actually expected behaviour. The @ parameter is a special case, with $0
being argv[0]. Normally, if you do "$@" it will expand to "$1" "$2" ... "$n",
without including $0. But in this case you're forcing it to expand from $0.
--
Eduardo Bustamante
https://dualbus.me/
Re: param expansion with single-character special vars in the environment
On 27 Apr 2016, at 20:43, Eduardo A. Bustamante López wrote: > The attached patch seems to take care of at least these two cases: [..] your patch also adresses the original Grisha's report: declare -r T=5; ref=$T; declare -n ref; ref=10; declare -n T; cheers, pg
Re: param expansion with single-character special vars in the environment
The attached patch seems to take care of at least these two cases:
| dualbus@hp ...src/gnu/bash % ./bash --norc --noprofile -c 'r=@; declare -n r'
| ./bash: line 0: declare: @: invalid variable name for name reference
|
| dualbus@hp ...src/gnu/bash % ./bash --norc --noprofile -c 'declare -n r; r=@'
| ./bash: `@': not a valid identifier
|
| dualbus@hp ...src/gnu/bash % ./bash --norc --noprofile -c 'declare -n r;
r=$(echo @)'
| ./bash: `@': not a valid identifier
But I think Chet already considered this, given that the check in declare.def
was '#if 0'ed out.
--
Eduardo Bustamante
https://dualbus.me/
diff --git a/builtins/declare.def b/builtins/declare.def
index a1e9b4e..2ae12a8 100644
--- a/builtins/declare.def
+++ b/builtins/declare.def
@@ -570,7 +570,7 @@ declare_internal (list, local_var)
}
else if (flags_on & att_nameref)
{
-#if 0
+#if 1
if (nameref_p (var) == 0 && var_isset (var) && var_isnull (var)
== 0 && legal_identifier (value_cell (var)) == 0)
{
builtin_error (_("%s: invalid variable name for name
reference"), value_cell (var));
diff --git a/variables.c b/variables.c
index 69ed170..dd62d96 100644
--- a/variables.c
+++ b/variables.c
@@ -2628,9 +2628,13 @@ bind_variable_internal (name, value, table, hflags,
aflags)
}
/* The first clause handles `declare -n ref; ref=x;' */
- if (entry && invisible_p (entry) && nameref_p (entry))
+ if (entry && invisible_p (entry) && nameref_p (entry)) {
+if(!legal_identifier(value)) {
+internal_error (_("`%s': not a valid identifier"), value);
+exit(1);
+}
goto assign_value;
- else if (entry && nameref_p (entry))
+ } else if (entry && nameref_p (entry))
{
newval = nameref_cell (entry);
#if defined (ARRAY_VARS)
Re: param expansion with single-character special vars in the environment
On Wed, Apr 27, 2016 at 08:41:20AM -0400, Grisha Levit wrote:
[...]
> The above works when the readonly variable has a value that is also a valid
> identifier. In my previous example I worked around this using the fact
> that ref=;
> declare -n ref does not check to make sure that $ref is a valid identifier.
[...]
It does seem to check, but only when you do: declare -n ref=
| dualbus@hp ...src/gnu/bash % gdb ./bash --batch -ex 'b sh_invalidid' -ex r
--args bash -c 'declare -n r=@'
| Breakpoint 1 at 0x493107: file common.c, line 236.
|
| Breakpoint 1, sh_invalidid (s=0x7dd26a "@") at common.c:236
| 236 builtin_error (_("`%s': not a valid identifier"), s);
|
| dualbus@hp ...src/gnu/bash % gdb ./bash --batch -ex 'b sh_invalidid' -ex r
--args bash -c 'declare -n r; r=@'
| Breakpoint 1 at 0x493107: file common.c, line 236.
| [Inferior 1 (process 5609) exited normally]
As you can see, in the first case, it hits the sh_invalidid breakpoint, and
bash actually complains about the invalid identifier.
--
Eduardo Bustamante
https://dualbus.me/
Re: param expansion with single-character special vars in the environment
Hi, On 27 Apr 2016, at 14:41, Grisha Levit wrote: > Once you convert the variable to a reference, you can control its value by > modifying the value of a different variable with the name that corresponds to > the value of the readonly variable, so “access to the referenced variable > should honor its attributes” probably won’t do much (If I understand your > suggestion correctly). well, now after having looked at the code I am not that smart anymore. I see your point, but on one hand you are creating this reference loop on purpose, yet still something should be forbidden here. I like the idea of the references less and less, especially at the current state of the affairs in the variable model the introduction of these constructs is risky. Maybe it is possible to do it in a smart way. cheers, pg
Re: Autocompletion problems with nounset and arrays
On 4/26/16 4:50 PM, Eric Pruitt wrote: > On Tue, Apr 26, 2016 at 01:54:12PM -0400, Chet Ramey wrote: >> Thanks for the report. I will fix this before bash-4.4 is released. > > Thank you. I am happy to compile a prerelease build to try out your > solution if you want another tester before you roll fix into an RC. It will show up in the next devel snapshot on savannah. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, ITS, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
Race condition in handling SIGHUP
Hello,
Recently we came across a bug in bash which was introduced by below patch :
http://git.savannah.gnu.org/cgit/bash.git/commit/?id=d5d00961
In bash 4.2 this could lead to a race condition. 'yy_readline_get ()' function
sets 'terminate_immediately' to 1 before calling readline() at [1].
If shell receives SIGHUP and executes 'termsig_handler ()' before setting
'terminate_immediately' back to 0 [2], we will end up at [3] (considering
'RL_ISSTATE (RL_STATE_READCMD)' evaluates to 0 when readline is not waiting to
read command from user), and ~/.bash_history will not be updated.
We started seeing this bug after above mentioned patch was backported to RHEL
6. Sometimes ~/.bash_history is not updated when user executes 'reboot' command
in a ssh session. This is caused by race condition in handling SIGHUP.
While this issue was fixed by backporting somes changes (See attached patch)
from [4] to bash-4.2 or older versions, there is still a corner case which may
cause race condition in handling SIGHUP in current upstream.
'bash_tilde_expand ()' function sets 'terminate_immediately' to 1 at [5]
If SIGHUP is received and termsig_handler () gets executed before reaching [6],
~/.bash_history will not be updated. This can happen in a scenario where user
is running ssh session and requests for expansion for '~', if an admin issues
'reboot' command at the same time then ~/.bash_history may not be updated.
I have 2 questions about how current upstream handles this condition :
1. Why 'terminate_immediately' is set to 1 at [7]?
2. Why 'RL_ISSTATE (RL_STATE_READCMD)' is being checked at [8]? This will
evaluate to 0 if readline is not waiting to read a command from user. I believe
this check can be removed.
[1] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1441
[2] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1446
[3]
http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c?id=d5d0096115d0d484fd669ad170498962ea45e841#n513
[4]
http://git.savannah.gnu.org/cgit/bash.git/commit/?id=ac50fbac377e32b98d2de396f016ea81e8ee9961
[5] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994
[6] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n1004
[7] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994
[8] http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c#n524
--
Siteshwar Vashisht
Software Engineer
From 1ecc0ff050d74aef548677fb97b2113b0920c711 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht
Date: Mon, 25 Apr 2016 14:04:10 +0530
Subject: [PATCH] Do not set terminate_immediately while reading input
---
builtins/read.def | 3 +--
parse.y | 5 +
y.tab.c | 5 +
3 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/builtins/read.def b/builtins/read.def
index 1ef9142..2dcaf35 100644
--- a/builtins/read.def
+++ b/builtins/read.def
@@ -455,7 +455,6 @@ read_builtin (list)
of the unwind-protect stack after the realloc() works right. */
add_unwind_protect (xfree, input_string);
interrupt_immediately++;
- terminate_immediately++;
unbuffered_read = (nchars > 0) || (delim != '\n') || input_is_pipe;
@@ -512,6 +511,7 @@ read_builtin (list)
if (retval <= 0)
{
+ CHECK_TERMSIG;
eof = 1;
break;
}
@@ -616,7 +616,6 @@ add_char:
zsyncfd (fd);
interrupt_immediately--;
- terminate_immediately--;
discard_unwind_frame ("read_builtin");
retval = eof ? EXECUTION_FAILURE : EXECUTION_SUCCESS;
diff --git a/parse.y b/parse.y
index 9a78d0c..0b9f9ff 100644
--- a/parse.y
+++ b/parse.y
@@ -1440,12 +1440,11 @@ yy_readline_get ()
old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
interrupt_immediately++;
}
- terminate_immediately = 1;
current_readline_line = readline (current_readline_prompt ?
current_readline_prompt : "");
- terminate_immediately = 0;
+ CHECK_TERMSIG;
if (signal_is_ignored (SIGINT) == 0 && old_sigint)
{
interrupt_immediately--;
@@ -1606,13 +1605,11 @@ yy_stream_get ()
if (interactive)
{
interrupt_immediately++;
- terminate_immediately++;
}
result = getc_with_restart (bash_input.location.file);
if (interactive)
{
interrupt_immediately--;
- terminate_immediately--;
}
}
return (result);
diff --git a/y.tab.c b/y.tab.c
index d702554..563d312 100644
--- a/y.tab.c
+++ b/y.tab.c
@@ -3757,12 +3757,11 @@ yy_readline_get ()
old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
interrupt_immediately++;
}
- terminate_immediately = 1;
current_readline_line = readline (current_readline_prompt ?
current_readline_prompt : "");
- terminate_immediately = 0;
+ CHECK_TERMSIG;
if (signal_is_ignored (SIGINT) == 0 && old_sigint)
{
interrupt_immediately--;
@@ -3923,13 +3922,11 @@ yy_stream_get ()
if (interactive)
{
Race condition in handling SIGHUP
Hello,
Recently we came across a bug in bash which was introduced by below patch :
http://git.savannah.gnu.org/cgit/bash.git/commit/?id=d5d00961
In bash 4.2 this could lead to a race condition. 'yy_readline_get ()' function
sets 'terminate_immediately' to 1 before calling readline() at [1].
If shell receives SIGHUP and executes 'termsig_handler ()' before setting
'terminate_immediately' back to 0 [2], we will end up at [3] (considering
'RL_ISSTATE (RL_STATE_READCMD)' evaluates to 0 when readline is not waiting to
read command from user), and ~/.bash_history will not be updated.
We started seeing this bug after above mentioned patch was backported to RHEL
6. Sometimes ~/.bash_history is not updated when user executes 'reboot' command
in a ssh session. This is caused by race condition in handling SIGHUP.
While this issue was fixed by backporting somes changes (See attached patch)
from [4] to bash-4.2 or older versions, there is still a corner case which may
cause race condition in handling SIGHUP in current upstream.
'bash_tilde_expand ()' function sets 'terminate_immediately' to 1 at [5]
If SIGHUP is received and termsig_handler () gets executed before reaching [6],
~/.bash_history will not be updated. This can happen in a scenario where user
is running ssh session and requests for expansion for '~', if an admin issues
'reboot' command at the same time then ~/.bash_history may not be updated.
I have 2 questions about how current upstream handles this condition :
1. Why 'terminate_immediately' is set to 1 at [7]?
2. Why 'RL_ISSTATE (RL_STATE_READCMD)' is being checked at [8]? This will
evaluate to 0 if readline is not waiting to read a command from user. I believe
this check can be removed.
[1] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1441
[2] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1446
[3]
http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c?id=d5d0096115d0d484fd669ad170498962ea45e841#n513
[4]
http://git.savannah.gnu.org/cgit/bash.git/commit/?id=ac50fbac377e32b98d2de396f016ea81e8ee9961
[5] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994
[6] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n1004
[7] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994
[8] http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c#n524
--
--
From 1ecc0ff050d74aef548677fb97b2113b0920c711 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht
Date: Mon, 25 Apr 2016 14:04:10 +0530
Subject: [PATCH] Do not set terminate_immediately while reading input
---
builtins/read.def | 3 +--
parse.y | 5 +
y.tab.c | 5 +
3 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/builtins/read.def b/builtins/read.def
index 1ef9142..2dcaf35 100644
--- a/builtins/read.def
+++ b/builtins/read.def
@@ -455,7 +455,6 @@ read_builtin (list)
of the unwind-protect stack after the realloc() works right. */
add_unwind_protect (xfree, input_string);
interrupt_immediately++;
- terminate_immediately++;
unbuffered_read = (nchars > 0) || (delim != '\n') || input_is_pipe;
@@ -512,6 +511,7 @@ read_builtin (list)
if (retval <= 0)
{
+ CHECK_TERMSIG;
eof = 1;
break;
}
@@ -616,7 +616,6 @@ add_char:
zsyncfd (fd);
interrupt_immediately--;
- terminate_immediately--;
discard_unwind_frame ("read_builtin");
retval = eof ? EXECUTION_FAILURE : EXECUTION_SUCCESS;
diff --git a/parse.y b/parse.y
index 9a78d0c..0b9f9ff 100644
--- a/parse.y
+++ b/parse.y
@@ -1440,12 +1440,11 @@ yy_readline_get ()
old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
interrupt_immediately++;
}
- terminate_immediately = 1;
current_readline_line = readline (current_readline_prompt ?
current_readline_prompt : "");
- terminate_immediately = 0;
+ CHECK_TERMSIG;
if (signal_is_ignored (SIGINT) == 0 && old_sigint)
{
interrupt_immediately--;
@@ -1606,13 +1605,11 @@ yy_stream_get ()
if (interactive)
{
interrupt_immediately++;
- terminate_immediately++;
}
result = getc_with_restart (bash_input.location.file);
if (interactive)
{
interrupt_immediately--;
- terminate_immediately--;
}
}
return (result);
diff --git a/y.tab.c b/y.tab.c
index d702554..563d312 100644
--- a/y.tab.c
+++ b/y.tab.c
@@ -3757,12 +3757,11 @@ yy_readline_get ()
old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
interrupt_immediately++;
}
- terminate_immediately = 1;
current_readline_line = readline (current_readline_prompt ?
current_readline_prompt : "");
- terminate_immediately = 0;
+ CHECK_TERMSIG;
if (signal_is_ignored (SIGINT) == 0 && old_sigint)
{
interrupt_immediately--;
@@ -3923,13 +3922,11 @@ yy_stream_get ()
if (interactive)
{
interrupt_immediately++;
-
Re: bash 4.4 and BASHOPTS containing extdebug in env
On 4/26/16 10:49 PM, Grisha Levit wrote: > Thanks for the explanation. I'd read that thread but thought the change > was supposed to only modify the behavior when explicitly specifying the > --debugger option. That also seems to be the gist of the original RedHat > bugzilla request that prompted the > thread: https://bugzilla.redhat.com/show_bug.cgi?id=1165793 That's the thing. They set the same option and enable the same behavior, and have always been tied together like that. It's kind of like --posix and -o posix. If that option is enabled when the shell starts up, the shell attempts to load the debugger start file, which creates the debugging environment. > Maybe the description of the extdebug option to the shopt builtin can be > modified with something like I'll add something. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, ITS, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
Global variable modification by nameref chain
Is the behavior below expected? In the presence of a local $var, the
*global* $var can be modified by assigning a value to a local nameref that
points to a global nameref that points to $var. However, the local nameref
expands to the *local* value of $var.
lref --> gref --> var
Example:
var=Gf() {
local var=L
declare -gn gref=var
declare -n lref=gref
echo "Writing 'F' to lref"
lref=F
echo "lref points to \$${!lref} and expands to '$lref'"
}echo "Global \$var expands to '$var'"
fecho "Global \$var expands to '$var'"
Global $var expands to *'G'*
Writing *'F'* to lref
lref points to $var and expands to *'L'*
Global $var expands to *'F'*
Re: param expansion with single-character special vars in the environment
On Wed, Apr 27, 2016 at 7:37 AM, Piotr Grzybowski wrote: It seems to me that creating the reference should be allowed, but the > access to the referenced variable should honor its attributes. > Once you convert the variable to a reference, you can control its value by modifying the value of a different variable with the name that corresponds to the value of the readonly variable, so “access to the referenced variable should honor its attributes” probably won’t do much (If I understand your suggestion correctly). In a less convoluted example that doesn’t rely on creating our own namerefs: readonly USER=sandbox USER=root # bash: USER: readonly variable declare -n USER sandbox=root# works USER=root # works [[ $USER == root ]] # 0 # USER is unchanged, other than the -n attributedeclare -p USER # declare -nrx USER="sandbox" -- The above works when the readonly variable has a value that is also a valid identifier. In my previous example I worked around this using the fact that ref=; declare -n ref does not check to make sure that $ref is a valid identifier. So: readonly PATH=/opt/bin ref=$PATHdeclare -n ref ref=/usr/bindeclare -p /opt/bin # declare -- /opt/bin="/usr/bin" declare -n PATH # declare -nr PATH="/opt/bin"echo $PATH # /usr/bin
Re: "${x-"$@"}" expansion
On Wed, Apr 27, 2016 at 07:33:25AM -0400, Grisha Levit wrote:
> Sorry that wasn???t very clear. I only included that case to demonstrate that
> seemingly contradictory things are happening:
>
>- "${_+$@}" expands each positional parameter to a separate word,
>following the usual "$@" behavior
To be honest, this looks like a bug in the script to me. The script
author intended ${_+"$@"} surely. Or more likely ${1+"$@"} which is
an actual feature used in legacy shell scripts.
The fact that "${_+$@}" and ${_+"$@"} both give the same thing when $@
is not empty seems like some sort of coincidence. Or, yet another trap
for the unwary. "Hey, I tested it and it worked!" ... 2 months later ...
"Shit, it didn't work in all cases."
P.S. dash and ksh93 also give this result.
>- The usual "$@" behavior is to expand to 0 words if there are no
>positional params but in this case "${_+$@}" expands to an empty string
>instead
Again, this looks like a bug in the script. I don't know what it's
supposed to do, but I know it should not have been written this way.
Also again, ${_+"$@"} gets it right.
Also also again, dash and ksh93 give the same result.
I'll leave the "what should the shell do when the script author makes
this particular bug" discussion to the rest of you. I just want to
advise people to stop writing bugs. Really, your life gets simpler.
(The fact that bash, dash and ksh93 all agree means it's probably the
prescribed POSIX behavior, but I can't be bothered to research that.
Hell, maybe all 3 shells just happen to have the same bug.)
Re: param expansion with single-character special vars in the environment
On 25 Apr 2016, at 22:57, Grisha Levit wrote: > A related issue is with adding the nameref attribute to a readonly variable. > Not sure if that should be allowed, as it can be used to change the meaning > of the variable [..] It seems to me that creating the reference should be allowed, but the access to the referenced variable should honor its attributes. cheers, pg
Re: "${x-"$@"}" expansion
Sorry that wasn’t very clear. I only included that case to demonstrate that
seemingly contradictory things are happening:
- "${_+$@}" expands each positional parameter to a separate word,
following the usual "$@" behavior
- The usual "$@" behavior is to expand to 0 words if there are no
positional params but in this case "${_+$@}" expands to an empty string
instead
- If we interpret "${_+$@}" as being equivalent to "${_+}" when there
are no params, then the two cases above seem reconcilable, but still
"${_+${_+$@}}" expanding to nothing does not make sense.
I suspect the below is not really the desired behavior?
fun() { echo $#; }
fun "$@"0
fun "${_+$@}"1
fun "${_+${_+$@}}"0
fun "${_+${_+${_+$@}}}"1
fun "${_+${_+${_+${_+$@"0
fun "${_+${_+${_+${_+${_+$@}"1
FWIW bash 4.1 and below treat all but the first case as one word.
Re: "${x-"$@"}" expansion
On 26 Apr 2016, at 21:03, Grisha Levit wrote:
> This behavior seems very strange. This example is with $@ but it seems the
> same for ${array[@]}
>
> The manual says for ${parameter:-word}:
>
> > If parameter is unset or null, the expansion of word is substituted.
>
> In this case, $@ is expanded as if it was quoted (even if 'word' is not
> quoted) and the outer quotes do no serve to quote the expansion of $@.
>
> $ set -- '1 1' '2 2'; unset x
> $ v=( "${x-$@}" ); declare -p v
> declare -a v=([0]="1 1" [1]="2 2")
I gather, you expect $* behavior?
#set -- '1 1' '2 2'; unset x
#v=( "${x-$*}" ); declare -p v
declare -a v='([0]="1 1 2 2")'
cheers,
pg
Re: Segfault assigning to nameref with bad array subscript
Hi Grisha,
confirmed.
I think this one fixes it:
diff --git a/variables.c b/variables.c
index 69ed170..9eeda46 100644
--- a/variables.c
+++ b/variables.c
@@ -2636,9 +2636,14 @@ bind_variable_internal (name, value, table, hflags,
aflags)
#if defined (ARRAY_VARS)
/* declare -n foo=x[2] */
if (valid_array_reference (newval, 0))
+{
/* XXX - should it be aflags? */
entry = assign_array_element (newval, make_variable_value (entry,
value, 0), aflags);
- else
+ if (entry == NULL)
+ {
+ return NULL;
+ }
+} else
#endif
{
entry = make_new_variable (newval, table);
cheers,
pg
On 27 Apr 2016, at 08:45, Grisha Levit wrote:
> Any of the following will crash bash:
>
> declare -n ref=a[*]; ref=
> declare -n ref=a[@]; ref=
> declare -n ref=a[-1]; a=(); ref=
>
> declare -A A; declare -n ref='A[$unset]'; ref=
>
> They all produce "bad array subscript" errors so could be caught.
>
> ==60597== Invalid read of size 4
> ==60597==at 0x100020BEE: bind_variable_internal (variables.c:2717)
> ==60597==by 0x1000392E3: do_assignment_internal (subst.c:3121)
> ==60597==by 0x10003F8D4: expand_word_list_internal (subst.c:3161)
> ==60597==by 0x100019094: execute_command_internal (execute_cmd.c:4105)
> ==60597==by 0x100017BF6: execute_command_internal (execute_cmd.c:2579)
> ==60597==by 0x10006A82E: parse_and_execute (evalstring.c:417)
> ==60597==by 0x132E7: run_one_command (in /Users/levit/utils/bin/bash)
> ==60597==by 0x12502: main (shell.c:724)
> ==60597== Address 0x28 is not stack'd, malloc'd or (recently) free'd
