Skip to content
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

Improved fix 'typeset -xu', 'typeset -xl' (rhbz#1188377) #147

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

hyenias
Copy link

@hyenias hyenias commented Oct 22, 2020

'typeset -xu' and 'typeset -xl' would export the variable but fail
to change case in the value as the check between old and new
attributes did not provide the necesssary insight for lower or
upper case transcoding due to the lower or upper case attribute
being set within typeset.c prior to calling name.c nv_newattr
function.

Previous rhbz#1188377 patch added a conditional check for size==-1
which in effect caused the nv_newattr export code block return
optimization to never be executed as one cannot set any attributes
using the readonly builtin. By altering the size==-1 check to !trans
the export only optimization can run.

Also, the rhbz#1188377 patch altered new_attr function by setting
the new size to oldsize if run by the readonly builtin. The result
of setting size==oldsize allowed the succeeding if statement to
run more frequently and if size was a non-zero value resulted in
nv_setsize resetting the value to what it already was. Investigation
yielded that size was always 0 coming from the readonly builtin.

src/cmd/ksh93/bltins/typeset.c:

  • Remove the setting of tdata.argnum to -1 as it is not needed due to
    existing name.c nv_newattr() logic.

src/cmd/ksh93/sh/name.c: nv_newattr():

  • Corrected the export only check optimization by using !trans instead
    of using size==-1.
  • Removed previous condition check to set size=oldsize if coming from
    the readonly builtin. nv_newattr already had existing logic to
    prevent changing the size via nv_setsize as size is always 0 when
    coming from readonly builtin.

'typeset -xu' and 'typeset -xl' would export the variable but fail
to change case in the value as the check between old and new
attributes did not provide the necesssary insight for lower or
upper case transcoding due to the lower or upper case attribute
being set within typeset.c prior to calling name.c nv_newattr
function.

Previous rhbz#1188377 patch added a conditional check for size==-1
which in effect caused the nv_newattr export code block return
optimization to never be executed as one cannot set any attributes
using the readonly builtin. By altering the size==-1 check to !trans
the export only optimization can run.

Also, the rhbz#1188377 patch altered new_attr function by setting
the new size to oldsize if run by the readonly builtin. The result
of setting size==oldsize allowed the succeeding if statement to
run more frequently and if size was a non-zero value resulted in
nv_setsize resetting the value to what it already was. Investigation
yielded that size was always 0 coming from the readonly builtin.

src/cmd/ksh93/bltins/typeset.c:
- Remove the setting of tdata.argnum to -1 as it is not needed due to
  existing name.c nv_newattr() logic.

src/cmd/ksh93/sh/name.c: nv_newattr():
- Corrected the export only check optimization by using !trans instead
  of using size==-1.
- Removed previous condition check to set size=oldsize if coming from
  the readonly builtin. nv_newattr already had existing logic to
  prevent changing the size via nv_setsize as size is always 0 when
  coming from readonly builtin.
hyenias referenced this pull request Oct 22, 2020
'typeset -xu' and 'typeset -xl' would export the variable but fail
to change case in the value under certain conditions.

Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-xufix.patch

This applies the patch essentially without change and adds a
regression test based on the reproducer provided in the RH bug.

Unfortunately there is no description of how the patch works and
it's a little obscure to me. As far as I can figure out, the cause
of the problem was that nv_newattr() erroneously processed a
nonexistent size option-argument such as what can be given to
options like typeset -F, e.g. typeset -F3 for 3 digits after the
dot. A nonexistent size argument is represented by the value of -1.
@McDutchie
Copy link

Thanks for the improvement. Merging. Sorry for the delay.

@McDutchie McDutchie merged commit 95fe07d into ksh93:master Nov 26, 2020
@hyenias hyenias deleted the fix-exporting-upper-lower-case branch November 28, 2020 15:32
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