* [PATCH] posix_builtins: allow exporting a reaonly
@ 2018-04-18 19:58 Martijn Dekker
2018-04-18 20:07 ` Bart Schaefer
2019-06-20 18:47 ` [PATCH] posix_builtins: allow exporting a readonly Martijn Dekker
0 siblings, 2 replies; 6+ messages in thread
From: Martijn Dekker @ 2018-04-18 19:58 UTC (permalink / raw)
To: Zsh hackers list
[-- Attachment #1: Type: text/plain, Size: 309 bytes --]
POSIX_BUILTINS incorrectly prohibits exporting a readonly variable. All
other POSIX shells allow this and there is nothing in the POSIX text[*]
that says it's not allowed. The attached patch fixes this.
Thanks,
- M.
[*]
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_22
[-- Attachment #2: export-readonly.patch --]
[-- Type: text/plain, Size: 456 bytes --]
diff --git a/Src/builtin.c b/Src/builtin.c
index 73cfe7a..a75c4b2 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2169,7 +2169,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
!ASG_VALUEP(asg))
on |= PM_UNSET;
else if (usepm && (pm->node.flags & PM_READONLY) &&
- !(on & PM_READONLY)) {
+ !(on & PM_READONLY) && !(on & PM_EXPORTED)) {
zerr("read-only variable: %s", pm->node.nam);
return NULL;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] posix_builtins: allow exporting a reaonly
2018-04-18 19:58 [PATCH] posix_builtins: allow exporting a reaonly Martijn Dekker
@ 2018-04-18 20:07 ` Bart Schaefer
2018-04-21 12:57 ` Martijn Dekker
2019-06-20 18:47 ` [PATCH] posix_builtins: allow exporting a readonly Martijn Dekker
1 sibling, 1 reply; 6+ messages in thread
From: Bart Schaefer @ 2018-04-18 20:07 UTC (permalink / raw)
To: Martijn Dekker; +Cc: Zsh hackers list
On Wed, Apr 18, 2018 at 12:58 PM, Martijn Dekker <martijn@inlv.org> wrote:
> POSIX_BUILTINS incorrectly prohibits exporting a readonly variable. All
> other POSIX shells allow this and there is nothing in the POSIX text[*] that
> says it's not allowed. The attached patch fixes this.
I hope it's really that simple -- see thread about typeset -T and
read-only and resulting confusion, e.g. zsh-workers/42061 and
references.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] posix_builtins: allow exporting a reaonly
2018-04-18 20:07 ` Bart Schaefer
@ 2018-04-21 12:57 ` Martijn Dekker
0 siblings, 0 replies; 6+ messages in thread
From: Martijn Dekker @ 2018-04-21 12:57 UTC (permalink / raw)
To: Bart Schaefer; +Cc: zsh-workers
Op 18-04-18 om 22:07 schreef Bart Schaefer:
> On Wed, Apr 18, 2018 at 12:58 PM, Martijn Dekker <martijn@inlv.org> wrote:
>> POSIX_BUILTINS incorrectly prohibits exporting a readonly variable. All
>> other POSIX shells allow this and there is nothing in the POSIX text[*] that
>> says it's not allowed. The attached patch fixes this.
>
> I hope it's really that simple -- see thread about typeset -T and
> read-only and resulting confusion, e.g. zsh-workers/42061 and
> references.
I can't really find anything in that thread or its references that seems
relevant.
Exporting readonly variables was already possible with POSIXBUILTINS
off, so I don't see how it could harm anything to allow it with
POSIXBUILTINS on.
Here is some more context than was provided by the patch. Note that all
of this is only executed if POSIXBUILTINS is on.
if (isset(POSIXBUILTINS)) {
/*
* Stricter rules about retaining readonly attribute in this case.
*/
if ((on & (PM_READONLY|PM_EXPORTED)) &&
(!usepm || (pm->node.flags & PM_UNSET)) &&
!ASG_VALUEP(asg))
on |= PM_UNSET;
else if (usepm && (pm->node.flags & PM_READONLY) &&
!(on & PM_READONLY) && !(on & PM_EXPORTED)) {
zerr("read-only variable: %s", pm->node.nam);
return NULL;
}
}
As the comment says, that block applies stricter rules about retaining
the readonly attribute for POSIXBUILTINS. I merely removed the
restriction that readonly variables cannot be exported by adding:
&& !(on & PM_EXPORTED)
Note that the restriction was already inconsistent: this code did not
block unset readonly variables from being given the export flag.
FWIW, the test suite passes cleanly with this patch.
Thanks,
- M.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] posix_builtins: allow exporting a readonly
2018-04-18 19:58 [PATCH] posix_builtins: allow exporting a reaonly Martijn Dekker
2018-04-18 20:07 ` Bart Schaefer
@ 2019-06-20 18:47 ` Martijn Dekker
2019-06-21 8:59 ` Peter Stephenson
1 sibling, 1 reply; 6+ messages in thread
From: Martijn Dekker @ 2019-06-20 18:47 UTC (permalink / raw)
To: zsh-workers
Op 18-04-18 om 20:58 schreef Martijn Dekker:
> POSIX_BUILTINS incorrectly prohibits exporting a readonly variable. All
> other POSIX shells allow this and there is nothing in the POSIX text[*]
> that says it's not allowed. The attached patch fixes this.
This seems to have been forgotten about, so I'm trying again.
To recap:
$ zsh -c 'readonly foo=123; export foo' # OK
$ zsh --emulate sh -c 'readonly foo=123; export foo' # BUG
zsh:1: read-only variable: foo
I had another look at it and I think this patch (attached) should be
even more straightforward. It simply checks if we are running the
'export' command and then reverts to normal behaviour instead of the
special-casing for POSIXBUILTINS which is incorrect for this case.
Note that the patched code block is only executed with POSIXBUILTINS
active so won't otherwise affect anything in any case.
Also, attempts to assign using 'export' still correctly error out with
the patch, as they do on other shells:
$ zsh --emulate sh -c 'readonly foo=123; export foo=123'
zsh:1: read-only variable: foo
Thanks,
- M.
--
modernish -- harness the shell
https://github.com/modernish/modernish
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] posix_builtins: allow exporting a readonly
2019-06-20 18:47 ` [PATCH] posix_builtins: allow exporting a readonly Martijn Dekker
@ 2019-06-21 8:59 ` Peter Stephenson
2019-06-22 11:54 ` Martijn Dekker
0 siblings, 1 reply; 6+ messages in thread
From: Peter Stephenson @ 2019-06-21 8:59 UTC (permalink / raw)
To: zsh-workers
On Thu, 2019-06-20 at 19:47 +0100, Martijn Dekker wrote:
> Op 18-04-18 om 20:58 schreef Martijn Dekker:
> >
> > POSIX_BUILTINS incorrectly prohibits exporting a readonly variable. All
> > other POSIX shells allow this and there is nothing in the POSIX text[*]
> > that says it's not allowed. The attached patch fixes this.
> This seems to have been forgotten about, so I'm trying again.
>
> To recap:
>
> $ zsh -c 'readonly foo=123; export foo' # OK
> $ zsh --emulate sh -c 'readonly foo=123; export foo' # BUG
> zsh:1: read-only variable: foo
>
> I had another look at it and I think this patch (attached) should be
> even more straightforward.
This sounds fine but I don't think there is an attachment.
pws
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] posix_builtins: allow exporting a readonly
2019-06-21 8:59 ` Peter Stephenson
@ 2019-06-22 11:54 ` Martijn Dekker
0 siblings, 0 replies; 6+ messages in thread
From: Martijn Dekker @ 2019-06-22 11:54 UTC (permalink / raw)
To: zsh-workers
[-- Attachment #1: Type: text/plain, Size: 817 bytes --]
Op 21-06-19 om 09:59 schreef Peter Stephenson:
> On Thu, 2019-06-20 at 19:47 +0100, Martijn Dekker wrote:
>> Op 18-04-18 om 20:58 schreef Martijn Dekker:
>>>
>>> POSIX_BUILTINS incorrectly prohibits exporting a readonly variable. All
>>> other POSIX shells allow this and there is nothing in the POSIX text[*]
>>> that says it's not allowed. The attached patch fixes this.
>> This seems to have been forgotten about, so I'm trying again.
>>
>> To recap:
>>
>> $ zsh -c 'readonly foo=123; export foo' # OK
>> $ zsh --emulate sh -c 'readonly foo=123; export foo' # BUG
>> zsh:1: read-only variable: foo
>>
>> I had another look at it and I think this patch (attached) should be
>> even more straightforward.
>
> This sounds fine but I don't think there is an attachment.
Woops. :-/
Here it is...
- M.
[-- Attachment #2: BUG_NOEXPRO.patch --]
[-- Type: text/plain, Size: 459 bytes --]
diff --git a/Src/builtin.c b/Src/builtin.c
index 2453f82c0..e863cc4bb 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2171,7 +2171,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
!ASG_VALUEP(asg))
on |= PM_UNSET;
else if (usepm && (pm->node.flags & PM_READONLY) &&
- !(on & PM_READONLY)) {
+ !(on & PM_READONLY) && func != BIN_EXPORT) {
zerr("read-only variable: %s", pm->node.nam);
return NULL;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-22 11:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-18 19:58 [PATCH] posix_builtins: allow exporting a reaonly Martijn Dekker
2018-04-18 20:07 ` Bart Schaefer
2018-04-21 12:57 ` Martijn Dekker
2019-06-20 18:47 ` [PATCH] posix_builtins: allow exporting a readonly Martijn Dekker
2019-06-21 8:59 ` Peter Stephenson
2019-06-22 11:54 ` Martijn Dekker
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).