zsh-workers
 help / color / mirror / code / Atom feed
* Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
@ 2015-10-15 13:54 Chi Hsuan Yen
  2015-10-15 20:54 ` Bart Schaefer
  0 siblings, 1 reply; 20+ messages in thread
From: Chi Hsuan Yen @ 2015-10-15 13:54 UTC (permalink / raw)
  To: zsh-workers

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

Dear Zsh developers:

This bug is similar to zsh-worker 36763 but different. With zsh commit
827d360, I have no problems in pasting CJK payloads with an empty ~/.zshrc,
while problems occur with my own ~/.zshrc.  It's a strange bug. Please tell
me if you can't reproduce it. I'll test on more platforms.

Environment:
OS:  Arch Linux x86_64
Compiler: gcc-multilib 5.2.0-2
Shell: zsh-git 5.1.1.r91.g827d360-1
This is built using the build script at [1]. Options given to ./configure
can be found there

I've also installed openbsd-netcat 1.105_7-7 for testing purpose

The minimal ~/.zshrc causing this bug:

autoload -Uz bracketed-paste-magic
zle -N bracketed-paste bracketed-paste-magic

zmodload zsh/net/tcp
ztcp localhost 12345

Steps to reproduce:
1. Install the packages mentioned above
2. Run 'nc -l 12345'
3. Copy "中文" in X11
I copied them from my browser, Firefox
4. Paste it in the terminal
I'm using xfce4-terminal. Ctrl+Shift+V does the trick.

Expected result:
"中文" is pasted

Actual result:
"df" is pasted

Notes:
1. If there's nothing listening at port 12345, ztcp fails as expected and
pasting CJK payloads works
2. Whether it's helpful or not, "中文" in UTF-8 is \xe4\xb8\xad\xe6\x96\x87

Best Regards,

Yen Chi Hsuan

[1] https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=zsh-git

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-15 13:54 Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads Chi Hsuan Yen
@ 2015-10-15 20:54 ` Bart Schaefer
  2015-10-16  0:25   ` Bart Schaefer
  0 siblings, 1 reply; 20+ messages in thread
From: Bart Schaefer @ 2015-10-15 20:54 UTC (permalink / raw)
  To: Chi Hsuan Yen; +Cc: Zsh hackers list

On Thu, Oct 15, 2015 at 6:54 AM, Chi Hsuan Yen <yan12125@gmail.com> wrote:
> This bug is similar to zsh-worker 36763 but different. With zsh commit
> 827d360, I have no problems in pasting CJK payloads with an empty ~/.zshrc,
> while problems occur with my own ~/.zshrc.  It's a strange bug. Please tell
> me if you can't reproduce it. I'll test on more platforms.

I am able to reproduce this, but not reliably.  Sometimes the paste
works as expected, sometimes not, though it seems to repeatedly fail
once it has failed for the first time.

If you are able to reproduce reliably, it would be interesting to
throw some diagnostics into bracketed-paste-magic, e.g. save the value
of $PASTED right after the call to "zle .bracketed-paste PASTED" and
see whether it's already wrong there or if "zle .read-command" is
getting back the wrong bytes later.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-15 20:54 ` Bart Schaefer
@ 2015-10-16  0:25   ` Bart Schaefer
  2015-10-16 19:40     ` Chi Hsuan Yen
  2015-10-28  2:43     ` Bart Schaefer
  0 siblings, 2 replies; 20+ messages in thread
From: Bart Schaefer @ 2015-10-16  0:25 UTC (permalink / raw)
  To: Chi Hsuan Yen, Zsh hackers list

On Oct 15,  1:54pm, Bart Schaefer wrote:
} Subject: Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted content
}
} On Thu, Oct 15, 2015 at 6:54 AM, Chi Hsuan Yen <yan12125@gmail.com> wrote:
} > This bug is similar to zsh-worker 36763 but different. With zsh commit
} > 827d360, I have no problems in pasting CJK payloads with an empty ~/.zshrc,
} > while problems occur with my own ~/.zshrc.  It's a strange bug. Please tell
} > me if you can't reproduce it. I'll test on more platforms.
} 
} I am able to reproduce this, but not reliably.

On the off chance that $(emulate) is somehow consuming some of the bytes
being pasted, try this?


diff --git a/Functions/Zle/bracketed-paste-magic b/Functions/Zle/bracketed-paste-magic
index cd4a708..2368bc3 100644
--- a/Functions/Zle/bracketed-paste-magic
+++ b/Functions/Zle/bracketed-paste-magic
@@ -116,10 +116,14 @@ quote-paste() {
 # Now the actual function
 
 bracketed-paste-magic() {
-    # Fast exit in the vi-mode cut-buffer context
     if [[ "$LASTWIDGET" = *vi-set-buffer ]]; then
+	# Fast exit in the vi-mode cut-buffer context
 	zle .bracketed-paste
 	return
+    else
+	# Capture the pasted text in $PASTED
+	local PASTED
+	zle .bracketed-paste PASTED
     fi
 
     # Really necessary to go to this much effort?
@@ -127,10 +131,9 @@ bracketed-paste-magic() {
 
     emulate -L zsh
     local -a bpm_hooks bpm_inactive
-    local PASTED bpm_func bpm_active bpm_keymap=$KEYMAP
+    local bpm_func bpm_active bpm_keymap=$KEYMAP
 
-    # Set PASTED and run the paste-init functions
-    zle .bracketed-paste PASTED
+    # Run the paste-init functions
     if zstyle -a :bracketed-paste-magic paste-init bpm_hooks; then
 	for bpm_func in $bpm_hooks; do
 	    if (( $+functions[$bpm_func] )); then


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-16  0:25   ` Bart Schaefer
@ 2015-10-16 19:40     ` Chi Hsuan Yen
  2015-10-18 15:52       ` Bart Schaefer
  2015-10-28  2:43     ` Bart Schaefer
  1 sibling, 1 reply; 20+ messages in thread
From: Chi Hsuan Yen @ 2015-10-16 19:40 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]

With a ubuntu 14.04 vagrant image [1] I can reproduce this bug all the time.

And the patch listed above works on neither ubuntu 14.04 nor Arch Linux.

[1]
https://cloud-images.ubuntu.com/vagrant/trusty/current/trusty-server-cloudimg-amd64-vagrant-disk1.box

On 16 October 2015 at 08:25, Bart Schaefer <schaefer@brasslantern.com>
wrote:

> On Oct 15,  1:54pm, Bart Schaefer wrote:
> } Subject: Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted
> content
> }
> } On Thu, Oct 15, 2015 at 6:54 AM, Chi Hsuan Yen <yan12125@gmail.com>
> wrote:
> } > This bug is similar to zsh-worker 36763 but different. With zsh commit
> } > 827d360, I have no problems in pasting CJK payloads with an empty
> ~/.zshrc,
> } > while problems occur with my own ~/.zshrc.  It's a strange bug. Please
> tell
> } > me if you can't reproduce it. I'll test on more platforms.
> }
> } I am able to reproduce this, but not reliably.
>
> On the off chance that $(emulate) is somehow consuming some of the bytes
> being pasted, try this?
>
>
> diff --git a/Functions/Zle/bracketed-paste-magic
> b/Functions/Zle/bracketed-paste-magic
> index cd4a708..2368bc3 100644
> --- a/Functions/Zle/bracketed-paste-magic
> +++ b/Functions/Zle/bracketed-paste-magic
> @@ -116,10 +116,14 @@ quote-paste() {
>  # Now the actual function
>
>  bracketed-paste-magic() {
> -    # Fast exit in the vi-mode cut-buffer context
>      if [[ "$LASTWIDGET" = *vi-set-buffer ]]; then
> +       # Fast exit in the vi-mode cut-buffer context
>         zle .bracketed-paste
>         return
> +    else
> +       # Capture the pasted text in $PASTED
> +       local PASTED
> +       zle .bracketed-paste PASTED
>      fi
>
>      # Really necessary to go to this much effort?
> @@ -127,10 +131,9 @@ bracketed-paste-magic() {
>
>      emulate -L zsh
>      local -a bpm_hooks bpm_inactive
> -    local PASTED bpm_func bpm_active bpm_keymap=$KEYMAP
> +    local bpm_func bpm_active bpm_keymap=$KEYMAP
>
> -    # Set PASTED and run the paste-init functions
> -    zle .bracketed-paste PASTED
> +    # Run the paste-init functions
>      if zstyle -a :bracketed-paste-magic paste-init bpm_hooks; then
>         for bpm_func in $bpm_hooks; do
>             if (( $+functions[$bpm_func] )); then
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-16 19:40     ` Chi Hsuan Yen
@ 2015-10-18 15:52       ` Bart Schaefer
  2015-10-18 16:19         ` Chi Hsuan Yen
  0 siblings, 1 reply; 20+ messages in thread
From: Bart Schaefer @ 2015-10-18 15:52 UTC (permalink / raw)
  To: Zsh hackers list

On Fri, Oct 16, 2015 at 12:40 PM, Chi Hsuan Yen <yan12125@gmail.com> wrote:
> With a ubuntu 14.04 vagrant image [1] I can reproduce this bug all the time.
>
> And the patch listed above works on neither ubuntu 14.04 nor Arch Linux.

I presume by that, you mean that the patch doesn't resolve your
problem, not that it breaks b-p-m in general?

Not surprising that it doesn't help, it was mostly intended to rule
out an unlikely cause.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-18 15:52       ` Bart Schaefer
@ 2015-10-18 16:19         ` Chi Hsuan Yen
  0 siblings, 0 replies; 20+ messages in thread
From: Chi Hsuan Yen @ 2015-10-18 16:19 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

On 18 October 2015 at 23:52, Bart Schaefer <schaefer@brasslantern.com>
wrote:

> On Fri, Oct 16, 2015 at 12:40 PM, Chi Hsuan Yen <yan12125@gmail.com>
> wrote:
> > With a ubuntu 14.04 vagrant image [1] I can reproduce this bug all the
> time.
> >
> > And the patch listed above works on neither ubuntu 14.04 nor Arch Linux.
>
> I presume by that, you mean that the patch doesn't resolve your
> problem, not that it breaks b-p-m in general?
>
> Not surprising that it doesn't help, it was mostly intended to rule
> out an unlikely cause.
>

Yep. bracketed-paste-magic works as usual. I've used the patched version of
Zsh for two days and nothing breaks except CJK payloads.

Best Regards,

Yen Chi Hsuan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-16  0:25   ` Bart Schaefer
  2015-10-16 19:40     ` Chi Hsuan Yen
@ 2015-10-28  2:43     ` Bart Schaefer
  2015-10-28  3:23       ` Bart Schaefer
  1 sibling, 1 reply; 20+ messages in thread
From: Bart Schaefer @ 2015-10-28  2:43 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Chi Hsuan Yen

On Oct 15,  1:54pm, Bart Schaefer wrote:
}
} On Thu, Oct 15, 2015 at 6:54 AM, Chi Hsuan Yen <yan12125@gmail.com> wrote:
} > This bug is similar to zsh-worker 36763 but different. With zsh
} > commit 827d360, I have no problems in pasting CJK payloads with an
} > empty ~/.zshrc, while problems occur with my own ~/.zshrc. It's a
} > strange bug. Please tell me if you can't reproduce it. I'll test
} > on more platforms.
} 
} I am able to reproduce this, but not reliably.

Fixing the descriptor "leak" in ztcp.c did not help with this bug, so
I tried digging around a bit further.

I first discovered that I'd mis-remembered what self-insert-unmeta is
meant for.  It's not un-metafying in the zsh sense, it's clearing the
high-order bit -- so it's the wrong thing for bracketed-paste-magic
to use when inserting characters, except maybe for ^M.

However, that led to the question of why that branch of the "case"
in bracketed-paste-magic is even being taken.  The code is:

                case $REPLY in
                    (${~bpm_active}) function () {
                        emulate -L $bpm_emulate; set -$bpm_opts
                        zle $REPLY
                    };;
                    (*) zle .self-insert-unmeta;;
                esac

(where $REPLY comes from "zle .read-command").  $REPLY is "self-insert"
and $bpm_active is "self-*" so the first branch ought to be taken, and
indeed that is what happens if ztcp has never been invoked.

However, if ztcp is run in the correct order with respect to the auto-
load of bracketed-paste-magic, the case statement goes wrong and the
(*) condition is taken instead.

This is eerily similar to a situation I mentioned some while ago in
which patterns in zstyle sometimes fail to match.  I've never been
able to consistently reproduce that one either, and it also seemed
to be dependent on the order in which some operations were done.

This has me entirely confused.  valgrind finds nothing amiss so it's
not a memory management thing.  Some sort of clash in global variable
address space?  Anybody have an idea?


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-28  2:43     ` Bart Schaefer
@ 2015-10-28  3:23       ` Bart Schaefer
  2015-10-28  9:35         ` Peter Stephenson
  0 siblings, 1 reply; 20+ messages in thread
From: Bart Schaefer @ 2015-10-28  3:23 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Chi Hsuan Yen

On Oct 27,  7:43pm, Bart Schaefer wrote:
}
} (where $REPLY comes from "zle .read-command").  $REPLY is "self-insert"
} and $bpm_active is "self-*" so the first branch ought to be taken, and
} indeed that is what happens if ztcp has never been invoked.
} 
} However, if ztcp is run in the correct order with respect to the auto-
} load of bracketed-paste-magic, the case statement goes wrong and the
} (*) condition is taken instead.

AHA!

tcp.c uses setiparam("REPLY"), so depending on whether $REPLY is already
defined or not, it gets initialized as an integer-typed variable, which
means that when "self-insert" is assigned to REPLY, it's treated as an
arithmetic expression and evaluates to 0.

tcp.c, socket.c, and zpty.c all use setiparam("REPLY"), potentially
inflicting this on other modules that later do setsparam("REPLY").  Yet
another reason for Kurtis to run for the hills.

Here's the bracketed-paste-magic repair, but we should consider whether
those modules should be setting REPLY as a string instead.


diff --git a/Functions/Zle/bracketed-paste-magic b/Functions/Zle/bracketed-paste-magic
index 2368bc3..2b2bc63 100644
--- a/Functions/Zle/bracketed-paste-magic
+++ b/Functions/Zle/bracketed-paste-magic
@@ -122,7 +122,7 @@ bracketed-paste-magic() {
 	return
     else
 	# Capture the pasted text in $PASTED
-	local PASTED
+	local PASTED REPLY
 	zle .bracketed-paste PASTED
     fi
 
@@ -170,14 +170,14 @@ bracketed-paste-magic() {
 	while [[ -n $PASTED ]] && zle .read-command; do
 	    PASTED=${PASTED#$KEYS}
 	    if [[ $KEYS = ${(~j:|:)${(b)bpm_inactive}} ]]; then
-		zle .self-insert-unmeta
+		zle .self-insert
 	    else
 		case $REPLY in
 		    (${~bpm_active}) function () {
 			emulate -L $bpm_emulate; set -$bpm_opts
 			zle $REPLY
 		    };;
-		    (*) zle .self-insert-unmeta;;
+		    (*) zle .self-insert;;
 		esac
 	    fi
 	done


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-28  3:23       ` Bart Schaefer
@ 2015-10-28  9:35         ` Peter Stephenson
  2015-10-28 17:07           ` Bart Schaefer
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Stephenson @ 2015-10-28  9:35 UTC (permalink / raw)
  To: Zsh hackers list

On Tue, 27 Oct 2015 20:23:40 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> AHA!
> 
> tcp.c uses setiparam("REPLY"), so depending on whether $REPLY is already
> defined or not, it gets initialized as an integer-typed variable, which
> means that when "self-insert" is assigned to REPLY, it's treated as an
> arithmetic expression and evaluates to 0.

I even noticed this the other day and got confused by it when playing
with ztcp myself, and nearly mentioned it.

I suspect the biggest problem is when you do stuff from the command line,
but it can obviously turn up anywhere.

> tcp.c, socket.c, and zpty.c all use setiparam("REPLY"), potentially
> inflicting this on other modules that later do setsparam("REPLY").  Yet
> another reason for Kurtis to run for the hills.

Or you could interpret it as a reason for warncreateglobal.  Does that
go off correctly if you set it?  If not, we might want to try to get it
to, even if we take the setitparam()s out.

I'd be inclined to get rid of this.  There's no good reason for using an
integer parameter for something that doesn't need to be involved in
arithmetic evaluation at all as it's going to spend its life just as a
number of no more than a couple of digits.

pws


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-28  9:35         ` Peter Stephenson
@ 2015-10-28 17:07           ` Bart Schaefer
  2015-10-28 17:44             ` Peter Stephenson
  0 siblings, 1 reply; 20+ messages in thread
From: Bart Schaefer @ 2015-10-28 17:07 UTC (permalink / raw)
  To: Zsh hackers list

On Oct 28,  9:35am, Peter Stephenson wrote:
} Subject: Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted content
}
} Or you could interpret it as a reason for warncreateglobal.  Does that
} go off correctly if you set it?  If not, we might want to try to get it
} to, even if we take the setitparam()s out.

Of course when run from the command prompt $REPLY is *correctly* being
set as a global, so there shouldn't be a warning.

However:

burner% zmodload zsh/net/tcp
burner% setopt warncreateglobal
burner% print $+REPLY
0
burner% () { ztcp localhost 12345 }
burner% print ${(t)REPLY}
integer
burner%           

So no warning, no.  setiparam() is coming in at too low a level.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-28 17:07           ` Bart Schaefer
@ 2015-10-28 17:44             ` Peter Stephenson
  2015-10-28 23:38               ` Bart Schaefer
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Stephenson @ 2015-10-28 17:44 UTC (permalink / raw)
  To: Zsh hackers list

On Wed, 28 Oct 2015 10:07:44 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Of course when run from the command prompt $REPLY is *correctly* being
> set as a global, so there shouldn't be a warning.
>
> However:
> 
> burner% zmodload zsh/net/tcp
> burner% setopt warncreateglobal
> burner% print $+REPLY
> 0
> burner% () { ztcp localhost 12345 }
> burner% print ${(t)REPLY}
> integer
> burner%
> 
> So no warning, no.  setiparam() is coming in at too low a level.

I wonder if the existing setsparam() etc. functions, which are currently
just aliases or front-ends for assignsparam() etc. with no warnings,
could be modified to add the flag based on the option?  That would save
a lot of mess in the following (although setiparam_no_convert() still
needs to be a separate function --- unless we decide setiparam() should
always do that but that's not obviously correct).

pws

diff --git a/Src/Modules/socket.c b/Src/Modules/socket.c
index f683496..c5adcd4 100644
--- a/Src/Modules/socket.c
+++ b/Src/Modules/socket.c
@@ -132,7 +132,9 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	/* allow to be closed explicitly */
 	fdtable[sfd] = FDT_EXTERNAL;
 
-	setiparam("REPLY", sfd);
+	assigniparam_no_convert(
+	    "REPLY", (zlong)sfd,
+	    isset(WARNCREATEGLOBAL) ? ASSPM_WARN_CREATE : 0);
 
 	if (verbose)
 	    printf("%s listener is on fd %d\n", soun.sun_path, sfd);
@@ -220,7 +222,9 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	    sfd = rfd;
 	}
 
-	setiparam("REPLY", sfd);
+	assigniparam_no_convert(
+	    "REPLY", (zlong)sfd,
+	    isset(WARNCREATEGLOBAL) ? ASSPM_WARN_CREATE : 0);
 
 	if (verbose)
 	    printf("new connection from %s is on fd %d\n", soun.sun_path, sfd);
@@ -261,7 +265,9 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 		fdtable[sfd] = FDT_EXTERNAL;
 	    }
 
-	    setiparam("REPLY", sfd);
+	    assigniparam_no_convert(
+		"REPLY", (zlong)sfd,
+		isset(WARNCREATEGLOBAL) ? ASSPM_WARN_CREATE : 0);
 
 	    if (verbose)
 		printf("%s is now on fd %d\n", soun.sun_path, sfd);
diff --git a/Src/Modules/tcp.c b/Src/Modules/tcp.c
index 7b0dcc7..b16d21c 100644
--- a/Src/Modules/tcp.c
+++ b/Src/Modules/tcp.c
@@ -461,7 +461,9 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func))
 	    return 1;
 	}
 
-	setiparam("REPLY", sess->fd);
+	assigniparam_no_convert(
+	    "REPLY", (zlong)sess->fd,
+	    isset(WARNCREATEGLOBAL) ? ASSPM_WARN_CREATE : 0);
 
 	if (verbose)
 	    printf("%d listener is on fd %d\n", ntohs(sess->sock.in.sin_port), sess->fd);
diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index 9741ee2..1100489 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -464,7 +464,9 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
 #endif
 	    errno == EINTR));
 
-    setiparam("REPLY", master);
+    assigniparam_no_convert(
+	"REPLY", (zlong)master,
+	isset(WARNCREATEGLOBAL) ? ASSPM_WARN_CREATE : 0);
 
     return 0;
 }
diff --git a/Src/params.c b/Src/params.c
index a8abb28..9ed998f 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -3025,6 +3025,24 @@ setiparam(char *s, zlong val)
     return setnparam(s, mnval);
 }
 
+/*
+ * Set an integer parameter without forcing creation of an integer type.
+ * This is useful if the integer is going to be set to a parmaeter which
+ * would usually be scalar but may not exist.
+ */
+
+/**/
+mod_export Param
+assigniparam_no_convert(char *s, zlong val, int flags)
+{
+    /*
+     * If the target is already an integer, thisgets converted
+     * back.  Low technology rules.
+     */
+    char buf[BDIGBUFSIZE];
+    convbase(buf, val, 10);
+    return assignsparam(s, ztrdup(buf), flags);
+}
 
 /* Unset a parameter */
 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-28 17:44             ` Peter Stephenson
@ 2015-10-28 23:38               ` Bart Schaefer
  2015-10-29  9:31                 ` Peter Stephenson
  0 siblings, 1 reply; 20+ messages in thread
From: Bart Schaefer @ 2015-10-28 23:38 UTC (permalink / raw)
  To: Zsh hackers list

On Oct 28,  5:44pm, Peter Stephenson wrote:
}
} I wonder if the existing setsparam() etc. functions, which are currently
} just aliases or front-ends for assignsparam() etc. with no warnings,
} could be modified to add the flag based on the option?

You mean modify the implementations, not the call signature, correct?

Testing of the WARNCREATEGLOBAL flag is pretty isolated at the moment
and there are a lot of calls to setsparam() et al.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-28 23:38               ` Bart Schaefer
@ 2015-10-29  9:31                 ` Peter Stephenson
  2015-10-29 14:51                   ` Peter Stephenson
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Stephenson @ 2015-10-29  9:31 UTC (permalink / raw)
  To: Zsh hackers list

On Wed, 28 Oct 2015 16:38:13 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Oct 28,  5:44pm, Peter Stephenson wrote:
> }
> } I wonder if the existing setsparam() etc. functions, which are currently
> } just aliases or front-ends for assignsparam() etc. with no warnings,
> } could be modified to add the flag based on the option?
> 
> You mean modify the implementations, not the call signature, correct?

Yes.

> Testing of the WARNCREATEGLOBAL flag is pretty isolated at the moment
> and there are a lot of calls to setsparam() et al.

Exactly.  I suspect they could all benefit --- I can't think of a case
where this is actually wrong, only where you don't care.  But
WARNCREATEGLOBAL isn't supposed to care whether your care, only warn...

pws


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-29  9:31                 ` Peter Stephenson
@ 2015-10-29 14:51                   ` Peter Stephenson
  2015-10-29 15:00                     ` Bart Schaefer
  2015-10-29 16:25                     ` Jun T.
  0 siblings, 2 replies; 20+ messages in thread
From: Peter Stephenson @ 2015-10-29 14:51 UTC (permalink / raw)
  To: Zsh hackers list

On Thu, 29 Oct 2015 09:31:31 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Wed, 28 Oct 2015 16:38:13 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > On Oct 28,  5:44pm, Peter Stephenson wrote:
> > }
> > } I wonder if the existing setsparam() etc. functions, which are currently
> > } just aliases or front-ends for assignsparam() etc. with no warnings,
> > } could be modified to add the flag based on the option?
> > 
> > You mean modify the implementations, not the call signature, correct?
> 
> Yes.

The key thing to get this not to be completely broken appears to be
updating the typeset code to call the assign functions with flags set to
0, i.e. what the replaced functions used to do.  As they were #define'd
this is exactly equivalent there.  Then other set*params() that aren't
within typeset will warn.

This is looking reasonably plausible --- a couple of bits of fallout in
add-zsh-hook and zsh-mime-setup already.  No doubt there will be more,
but probably I'll need to commit it to track them down.

Includes the effect of the change not to integerise REPLY unnecessarily.

diff --git a/Functions/MIME/zsh-mime-setup b/Functions/MIME/zsh-mime-setup
index 23e44fd..35f6e6b 100644
--- a/Functions/MIME/zsh-mime-setup
+++ b/Functions/MIME/zsh-mime-setup
@@ -1,7 +1,7 @@
 emulate -L zsh
 setopt extendedglob cbases
 
-local opt o_verbose o_list
+local opt o_verbose o_list i
 
 autoload -Uz zsh-mime-handler
 
diff --git a/Functions/Misc/add-zsh-hook b/Functions/Misc/add-zsh-hook
index ee37d67..fc39659 100644
--- a/Functions/Misc/add-zsh-hook
+++ b/Functions/Misc/add-zsh-hook
@@ -82,9 +82,11 @@ if (( del )); then
 else
   if (( ${(P)+hook} )); then
     if (( ${${(P)hook}[(I)$fn]} == 0 )); then
+      typeset -ga $hook
       set -A $hook ${(P)hook} $fn
     fi
   else
+    typeset -ga $hook
     set -A $hook $fn
   fi
   autoload $autoopts -- $fn
diff --git a/Src/Modules/socket.c b/Src/Modules/socket.c
index f683496..7c3fb5e 100644
--- a/Src/Modules/socket.c
+++ b/Src/Modules/socket.c
@@ -132,7 +132,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	/* allow to be closed explicitly */
 	fdtable[sfd] = FDT_EXTERNAL;
 
-	setiparam("REPLY", sfd);
+	setiparam_no_convert("REPLY", (zlong)sfd);
 
 	if (verbose)
 	    printf("%s listener is on fd %d\n", soun.sun_path, sfd);
@@ -220,7 +220,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	    sfd = rfd;
 	}
 
-	setiparam("REPLY", sfd);
+	setiparam_no_convert("REPLY", (zlong)sfd);
 
 	if (verbose)
 	    printf("new connection from %s is on fd %d\n", soun.sun_path, sfd);
@@ -261,7 +261,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 		fdtable[sfd] = FDT_EXTERNAL;
 	    }
 
-	    setiparam("REPLY", sfd);
+	    setiparam_no_convert("REPLY", (zlong)sfd);
 
 	    if (verbose)
 		printf("%s is now on fd %d\n", soun.sun_path, sfd);
diff --git a/Src/Modules/tcp.c b/Src/Modules/tcp.c
index 7b0dcc7..9fc1b29 100644
--- a/Src/Modules/tcp.c
+++ b/Src/Modules/tcp.c
@@ -461,7 +461,7 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func))
 	    return 1;
 	}
 
-	setiparam("REPLY", sess->fd);
+	setiparam_no_convert("REPLY", (zlong)sess->fd);
 
 	if (verbose)
 	    printf("%d listener is on fd %d\n", ntohs(sess->sock.in.sin_port), sess->fd);
@@ -562,7 +562,7 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func))
 	    sess->fd = rfd;
 	}
 
-	setiparam("REPLY", sess->fd);
+	setiparam_no_convert("REPLY", (zlong)sess->fd);
 
 	if (verbose)
 	    printf("%d is on fd %d\n", ntohs(sess->peer.in.sin_port), sess->fd);
@@ -681,7 +681,7 @@ bin_ztcp(char *nam, char **args, Options ops, UNUSED(int func))
 		}
 	    }
 
-	    setiparam("REPLY", sess->fd);
+	    setiparam_no_convert("REPLY", (zlong)sess->fd);
 
 	    if (verbose)
 		printf("%s:%d is now on fd %d\n",
diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index 9741ee2..3b83660 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -464,7 +464,7 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
 #endif
 	    errno == EINTR));
 
-    setiparam("REPLY", master);
+    setiparam_no_convert("REPLY", (zlong)master);
 
     return 0;
 }
diff --git a/Src/builtin.c b/Src/builtin.c
index 97022ad..8045bc8 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2090,7 +2090,9 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 			tc = 0;	/* but don't do a normal conversion */
 		    }
 		} else if (!setsecondstype(pm, on, off)) {
-		    if (asg->value.scalar && !(pm = setsparam(pname, ztrdup(asg->value.scalar))))
+		    if (asg->value.scalar &&
+			!(pm = assignsparam(
+			      pname, ztrdup(asg->value.scalar), 0)))
 			return NULL;
 		    usepm = 1;
 		    err = 0;
@@ -2202,12 +2204,13 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	    } else if (pm->env && !(pm->node.flags & PM_HASHELEM))
 		delenv(pm);
 	    DPUTS(ASG_ARRAYP(asg), "BUG: typeset got array value where scalar expected");
-	    if (asg->value.scalar && !(pm = setsparam(pname, ztrdup(asg->value.scalar))))
+	    if (asg->value.scalar &&
+		!(pm = assignsparam(pname, ztrdup(asg->value.scalar), 0)))
 		return NULL;
 	} else if (asg->is_array) {
-	    if (!(pm = setaparam(pname, asg->value.array ?
+	    if (!(pm = assignaparam(pname, asg->value.array ?
 				 zlinklist2array(asg->value.array) :
-				 mkarray(NULL))))
+				 mkarray(NULL), 0)))
 		return NULL;
 	}
 	pm->node.flags |= (on & PM_READONLY);
@@ -2347,16 +2350,18 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	     * creating a stray parameter along the way via createparam(),
 	     * now called below in the isident() branch.
 	     */
-	    if (!(pm = setsparam(pname, ztrdup(asg->value.scalar ? asg->value.scalar : ""))))
+	    if (!(pm = assignsparam(
+		      pname,
+		      ztrdup(asg->value.scalar ? asg->value.scalar : ""), 0)))
 		return NULL;
 	    dont_set = 1;
 	    asg->is_array = 0;
 	    keeplocal = 0;
 	    on = pm->node.flags;
 	} else if (PM_TYPE(on) == PM_ARRAY && ASG_ARRAYP(asg)) {
-	    if (!(pm = setaparam(pname, asg->value.array ?
-				 zlinklist2array(asg->value.array) :
-				 mkarray(NULL))))
+	    if (!(pm = assignaparam(pname, asg->value.array ?
+				    zlinklist2array(asg->value.array) :
+				    mkarray(NULL), 0)))
 		return NULL;
 	    dont_set = 1;
 	    keeplocal = 0;
@@ -2433,13 +2438,13 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	Param ipm = pm;
 	if (pm->node.flags & (PM_ARRAY|PM_HASHED)) {
 	    DPUTS(!ASG_ARRAYP(asg), "BUG: inconsistent scalar value for array");
-	    if (!(pm=setaparam(pname, asg->value.array ?
-			       zlinklist2array(asg->value.array) :
-			       mkarray(NULL))))
+	    if (!(pm=assignaparam(pname, asg->value.array ?
+				  zlinklist2array(asg->value.array) :
+				  mkarray(NULL), 0)))
 		return NULL;
 	} else {
 	    DPUTS(ASG_ARRAYP(asg), "BUG: inconsistent array value for scalar");
-	    if (!(pm = setsparam(pname, ztrdup(asg->value.scalar))))
+	    if (!(pm = assignsparam(pname, ztrdup(asg->value.scalar), 0)))
 		return NULL;
 	}
 	if (pm != ipm) {
@@ -2687,9 +2692,10 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
 		    /* Update join character */
 		    tdp->joinchar = joinchar;
 		    if (asg0.value.scalar)
-			setsparam(asg0.name, ztrdup(asg0.value.scalar));
+			assignsparam(asg0.name, ztrdup(asg0.value.scalar), 0);
 		    else if (asg->value.array)
-			setaparam(asg->name, zlinklist2array(asg->value.array));
+			assignaparam(
+			    asg->name, zlinklist2array(asg->value.array), 0);
 		    return 0;
 		} else {
 		    zwarnnam(name, "can't tie already tied scalar: %s",
@@ -2750,9 +2756,9 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
 	    zsfree(apm->ename);
 	apm->ename = ztrdup(asg0.name);
 	if (asg->value.array)
-	    setaparam(asg->name, zlinklist2array(asg->value.array));
+	    assignaparam(asg->name, zlinklist2array(asg->value.array), 0);
 	else if (oldval)
-	    setsparam(asg0.name, oldval);
+	    assignsparam(asg0.name, oldval, 0);
 	unqueue_signals();
 
 	return 0;
diff --git a/Src/params.c b/Src/params.c
index a8abb28..4d33660 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2828,6 +2828,15 @@ assignsparam(char *s, char *val, int flags)
 
 /**/
 mod_export Param
+setsparam(char *s, char *val)
+{
+    return assignsparam(
+	s, val, isset(WARNCREATEGLOBAL) && locallevel > 0 ?
+	ASSPM_WARN_CREATE : 0);
+}
+
+/**/
+mod_export Param
 assignaparam(char *s, char **val, int flags)
 {
     struct value vbuf;
@@ -2914,6 +2923,16 @@ assignaparam(char *s, char **val, int flags)
     return v->pm;
 }
 
+
+/**/
+mod_export Param
+setaparam(char *s, char **aval)
+{
+    return assignaparam(
+	s, aval, isset(WARNCREATEGLOBAL) && locallevel >0 ?
+	ASSPM_WARN_CREATE : 0);
+}
+
 /**/
 mod_export Param
 sethparam(char *s, char **val)
@@ -2937,11 +2956,15 @@ sethparam(char *s, char **val)
     if (unset(EXECOPT))
 	return NULL;
     queue_signals();
-    if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING)))
+    if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING))) {
 	createparam(t, PM_HASHED);
-    else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) &&
+	if (isset(WARNCREATEGLOBAL) && locallevel > 0 && v->pm->level == 0)
+	    zwarn("associative array parameter %s created globally in function",
+		  v->pm->node.nam);
+    } else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) &&
 	     !(v->pm->node.flags & PM_SPECIAL)) {
 	unsetparam(t);
+	/* no WARNCREATEGLOBAL check here as parameter already existed */
 	createparam(t, PM_HASHED);
 	v = NULL;
     }
@@ -2968,6 +2991,7 @@ setnparam(char *s, mnumber val)
     Value v;
     char *t = s, *ss;
     Param pm;
+    int was_unset = 0;
 
     if (!isident(s)) {
 	zerr("not an identifier: %s", s);
@@ -2987,6 +3011,7 @@ setnparam(char *s, mnumber val)
 	 */
 	unset(KSHARRAYS) && !ss) {
 	unsetparam_pm(v->pm, 0, 1);
+	was_unset = 1;
 	s = t;
 	v = NULL;
     }
@@ -3007,6 +3032,10 @@ setnparam(char *s, mnumber val)
 	}
 	v = getvalue(&vbuf, &t, 1);
 	DPUTS(!v, "BUG: value not found for new parameter");
+	if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > 0 &&
+	    v->pm->level == 0)
+	    zwarn("numeric parameter %s created globally in function",
+		  v->pm->node.nam);
     }
     setnumvalue(v, val);
     unqueue_signals();
@@ -3025,6 +3054,26 @@ setiparam(char *s, zlong val)
     return setnparam(s, mnval);
 }
 
+/*
+ * Set an integer parameter without forcing creation of an integer type.
+ * This is useful if the integer is going to be set to a parmaeter which
+ * would usually be scalar but may not exist.
+ */
+
+/**/
+mod_export Param
+setiparam_no_convert(char *s, zlong val)
+{
+    /*
+     * If the target is already an integer, thisgets converted
+     * back.  Low technology rules.
+     */
+    char buf[BDIGBUFSIZE];
+    convbase(buf, val, 10);
+    return assignsparam(
+	s, ztrdup(buf),
+	isset(WARNCREATEGLOBAL) && locallevel > 0 ? ASSPM_WARN_CREATE : 0);
+}
 
 /* Unset a parameter */
 
diff --git a/Src/zsh.h b/Src/zsh.h
index f819249..a6f0397 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1939,9 +1939,6 @@ struct paramdef {
     { name, flags | PM_SPECIAL | PM_HIDE | PM_HIDEVAL, \
 	    NULL, gsufn, getfn, scanfn, NULL }
 
-#define setsparam(S,V) assignsparam(S,V,0)
-#define setaparam(S,V) assignaparam(S,V,0)
-
 /*
  * Flags for assignsparam and assignaparam.
  */


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-29 14:51                   ` Peter Stephenson
@ 2015-10-29 15:00                     ` Bart Schaefer
  2015-10-29 15:10                       ` Peter Stephenson
  2015-10-29 16:25                     ` Jun T.
  1 sibling, 1 reply; 20+ messages in thread
From: Bart Schaefer @ 2015-10-29 15:00 UTC (permalink / raw)
  To: Zsh hackers list

On Oct 29,  2:51pm, Peter Stephenson wrote:
}
} The key thing to get this not to be completely broken appears to be
} updating the typeset code to call the assign functions with flags set to
} 0, i.e. what the replaced functions used to do.

Maybe this is a silly question, but do we ever use ASSPM_WARN_CREATE in
a case where WARNCREATEGLOBAL is *not* set?

Maybe the right thing to do is test WARNCREATEGLOBAL all the way at
the bottom, and have the typeset code call the assign functions with
an ASSPM_CREATE_SILENT flag, i.e., get rid of ASSPM_WARN_CREATE.

Then the set?param() functions don't have to change at all?  Or am I
missing something?


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-29 15:00                     ` Bart Schaefer
@ 2015-10-29 15:10                       ` Peter Stephenson
  0 siblings, 0 replies; 20+ messages in thread
From: Peter Stephenson @ 2015-10-29 15:10 UTC (permalink / raw)
  To: Zsh hackers list

On Thu, 29 Oct 2015 08:00:52 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Oct 29,  2:51pm, Peter Stephenson wrote:
> }
> } The key thing to get this not to be completely broken appears to be
> } updating the typeset code to call the assign functions with flags set to
> } 0, i.e. what the replaced functions used to do.
> 
> Maybe this is a silly question, but do we ever use ASSPM_WARN_CREATE in
> a case where WARNCREATEGLOBAL is *not* set?

I doubt it.

> Maybe the right thing to do is test WARNCREATEGLOBAL all the way at
> the bottom, and have the typeset code call the assign functions with
> an ASSPM_CREATE_SILENT flag, i.e., get rid of ASSPM_WARN_CREATE.
> 
> Then the set?param() functions don't have to change at all?  Or am I
> missing something?

That sounds like it ought to work --- except that to avoid the extra
cases I added to the two set*param functions that weren't simple aliases
we'd have to work out carefullly where "all the way at the bottom"
actually is.

pws


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-29 14:51                   ` Peter Stephenson
  2015-10-29 15:00                     ` Bart Schaefer
@ 2015-10-29 16:25                     ` Jun T.
  2015-10-29 16:56                       ` Peter Stephenson
  1 sibling, 1 reply; 20+ messages in thread
From: Jun T. @ 2015-10-29 16:25 UTC (permalink / raw)
  To: zsh-workers


2015/10/29 23:51, Peter Stephenson <p.stephenson@samsung.com> wrote:
> @@ -3007,6 +3032,10 @@ setnparam(char *s, mnumber val)
> 
> 	DPUTS(!v, "BUG: value not found for new parameter");
> +	if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > 0 &&
> +	    v->pm->level == 0)
> +	    zwarn("numeric parameter %s created globally in function",
> +		  v->pm->node.nam);

E01options.ztst also need be updated; otherwise it fails as follows:

*** /tmp/zsh.ztst.err.65239	Fri Oct 30 00:54:52 2015
--- /tmp/zsh.ztst.terr.65239	Fri Oct 30 00:54:52 2015
***************
*** 1,3 ****
--- 1,4 ----
  fn:3: scalar parameter foo1 created globally in function
  fn:5: scalar parameter foo1 created globally in function
  fn:15: math parameter foo5 created globally in function fn
+ fn:15: numeric parameter foo5 created globally in function
Test ./E01options.ztst failed: error output differs from expected as shown above for:
  fn() {
    emulate -L zsh
    setopt warncreateglobal
(snip)
  }
  fn
Was testing: WARN_CREATE_GLOBAL option



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-29 16:25                     ` Jun T.
@ 2015-10-29 16:56                       ` Peter Stephenson
  2015-10-30 15:02                         ` Daniel Shahaf
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Stephenson @ 2015-10-29 16:56 UTC (permalink / raw)
  To: zsh-workers

On Fri, 30 Oct 2015 01:25:49 +0900
Jun T. <takimoto-j@kba.biglobe.ne.jp> wrote:
> E01options.ztst also need be updated; otherwise it fails as follows:
> 
> *** /tmp/zsh.ztst.err.65239	Fri Oct 30 00:54:52 2015
> --- /tmp/zsh.ztst.terr.65239	Fri Oct 30 00:54:52 2015
> ***************
> *** 1,3 ****
> --- 1,4 ----
>   fn:3: scalar parameter foo1 created globally in function
>   fn:5: scalar parameter foo1 created globally in function
>   fn:15: math parameter foo5 created globally in function fn
> + fn:15: numeric parameter foo5 created globally in function

Hmmm... I must have been asleep when the math test was added, since
there's no reason at all why it should be inconsistent with the others.
Either report the function or not.

This ports the code to report the function.  The math-specific code is
no longer needed now we have a check in setnparam().  I've been
overcautious since I can't see why we'd fail to find a function
on the trace stack if locallevel is non-zero.

pws

diff --git a/Src/math.c b/Src/math.c
index eee21e1..37981cf 100644
--- a/Src/math.c
+++ b/Src/math.c
@@ -893,25 +893,6 @@ getcvar(char *s)
     return mn;
 }
 
-
-/* If script execution is inside a function call that hasn't returned,
- * return the name of that function.  Else return NULL.
- */
-
-/**/
-static const char *
-in_function_call(void)
-{
-    Funcstack i;
-    for (i = funcstack; i; i = i->prev)
-	if (i->tp == FS_FUNC) {
-	    DPUTS(!i->name, "funcstack entry with no name");
-	    return i->name;
-	}
-
-    return NULL;
-}
-
 /**/
 static mnumber
 setmathvar(struct mathvalue *mvp, mnumber v)
@@ -947,13 +928,6 @@ setmathvar(struct mathvalue *mvp, mnumber v)
     if (noeval)
 	return v;
     untokenize(mvp->lval);
-    if (isset(WARNCREATEGLOBAL)) {
-	const char *function_name;
-	if (!paramtab->getnode(paramtab, mvp->lval) &&
-	    (function_name = in_function_call()))
-	    zwarn("math parameter %s created globally in function %s",
-		  mvp->lval, function_name);
-    }
     pm = setnparam(mvp->lval, v);
     if (pm) {
 	/*
diff --git a/Src/params.c b/Src/params.c
index 4d33660..5058695 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2695,6 +2695,37 @@ gethkparam(char *s)
 }
 
 /**/
+static void
+check_warn_create(Param pm, const char *pmtype)
+{
+    Funcstack i;
+    const char *name;
+
+    if (pm->level != 0)
+	return;
+
+    name = NULL;
+    for (i = funcstack; i; i = i->prev) {
+	if (i->tp == FS_FUNC) {
+	    DPUTS(!i->name, "funcstack entry with no name");
+	    name = i->name;
+	    break;
+	}
+    }
+
+    if (name)
+    {
+	zwarn("%s parameter %s created globally in function %s",
+	      pmtype, pm->node.nam, name);
+    }
+    else
+    {
+	zwarn("%s parameter %s created globally in function",
+	      pmtype, pm->node.nam);
+    }
+}
+
+/**/
 mod_export Param
 assignsparam(char *s, char *val, int flags)
 {
@@ -2747,9 +2778,8 @@ assignsparam(char *s, char *val, int flags)
 	zsfree(val);
 	return NULL;
     }
-    if ((flags & ASSPM_WARN_CREATE) && v->pm->level == 0)
-	zwarn("scalar parameter %s created globally in function",
-	      v->pm->node.nam);
+    if (flags & ASSPM_WARN_CREATE)
+	check_warn_create(v->pm, "scalar");
     if (flags & ASSPM_AUGMENT) {
 	if (v->start == 0 && v->end == -1) {
 	    switch (PM_TYPE(v->pm->node.flags)) {
@@ -2898,9 +2928,8 @@ assignaparam(char *s, char **val, int flags)
 	    return NULL;
 	}
 
-    if ((flags & ASSPM_WARN_CREATE) && v->pm->level == 0)
-	zwarn("array parameter %s created globally in function",
-	      v->pm->node.nam);
+    if (flags & ASSPM_WARN_CREATE)
+	check_warn_create(v->pm, "array");
     if (flags & ASSPM_AUGMENT) {
     	if (v->start == 0 && v->end == -1) {
 	    if (PM_TYPE(v->pm->node.flags) & PM_ARRAY) {
@@ -2958,9 +2987,8 @@ sethparam(char *s, char **val)
     queue_signals();
     if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING))) {
 	createparam(t, PM_HASHED);
-	if (isset(WARNCREATEGLOBAL) && locallevel > 0 && v->pm->level == 0)
-	    zwarn("associative array parameter %s created globally in function",
-		  v->pm->node.nam);
+	if (isset(WARNCREATEGLOBAL) && locallevel > 0)
+	    check_warn_create(v->pm, "associative array");
     } else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) &&
 	     !(v->pm->node.flags & PM_SPECIAL)) {
 	unsetparam(t);
@@ -3032,10 +3060,8 @@ setnparam(char *s, mnumber val)
 	}
 	v = getvalue(&vbuf, &t, 1);
 	DPUTS(!v, "BUG: value not found for new parameter");
-	if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > 0 &&
-	    v->pm->level == 0)
-	    zwarn("numeric parameter %s created globally in function",
-		  v->pm->node.nam);
+	if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > 0)
+	    check_warn_create(v->pm, "numeric parameter");
     }
     setnumvalue(v, val);
     unqueue_signals();
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 1caee8d..15468e8 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -1116,9 +1116,9 @@
   }
   fn
 0:WARN_CREATE_GLOBAL option
-?fn:3: scalar parameter foo1 created globally in function
-?fn:5: scalar parameter foo1 created globally in function
-?fn:15: math parameter foo5 created globally in function fn
+?fn:3: scalar parameter foo1 created globally in function fn
+?fn:5: scalar parameter foo1 created globally in function fn
+?fn:15: numeric parameter parameter foo5 created globally in function fn
 
 # This really just tests if XTRACE is egregiously broken.
 # To test it properly would need a full set of its own.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-29 16:56                       ` Peter Stephenson
@ 2015-10-30 15:02                         ` Daniel Shahaf
  2015-11-03 16:31                           ` Chi Hsuan Yen
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Shahaf @ 2015-10-30 15:02 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

Peter Stephenson wrote on Thu, Oct 29, 2015 at 16:56:25 +0000:
> On Fri, 30 Oct 2015 01:25:49 +0900
> Jun T. <takimoto-j@kba.biglobe.ne.jp> wrote:
> > E01options.ztst also need be updated; otherwise it fails as follows:
> > 
> > *** /tmp/zsh.ztst.err.65239	Fri Oct 30 00:54:52 2015
> > --- /tmp/zsh.ztst.terr.65239	Fri Oct 30 00:54:52 2015
> > ***************
> > *** 1,3 ****
> > --- 1,4 ----
> >   fn:3: scalar parameter foo1 created globally in function
> >   fn:5: scalar parameter foo1 created globally in function
> >   fn:15: math parameter foo5 created globally in function fn
> > + fn:15: numeric parameter foo5 created globally in function
> 
> Hmmm... I must have been asleep when the math test was added, since
> there's no reason at all why it should be inconsistent with the others.
> Either report the function or not.
> 
> -?fn:3: scalar parameter foo1 created globally in function
> -?fn:5: scalar parameter foo1 created globally in function
> -?fn:15: math parameter foo5 created globally in function fn
> +?fn:3: scalar parameter foo1 created globally in function fn
> +?fn:5: scalar parameter foo1 created globally in function fn
> +?fn:15: numeric parameter parameter foo5 created globally in function fn

Thanks for improving it ☺

I'll fix the "numeric parameter parameter" thing.

> This ports the code to report the function.  The math-specific code is
> no longer needed now we have a check in setnparam().  I've been
> overcautious since I can't see why we'd fail to find a function
> on the trace stack if locallevel is non-zero.

I can't see when that would happen, either; the reason for the NULL
checks might be, more than anything else, that I wasn't familiar with
the C funcstack data structure.

Thanks,

Daniel


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads
  2015-10-30 15:02                         ` Daniel Shahaf
@ 2015-11-03 16:31                           ` Chi Hsuan Yen
  0 siblings, 0 replies; 20+ messages in thread
From: Chi Hsuan Yen @ 2015-11-03 16:31 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Peter Stephenson, Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 2262 bytes --]

Dear Zsh developers,

Today I've built Zsh again with commit 9642aeeaebd654565a045475d4d3ba101bfaec8f
and it works like a charm. CJK payloads can be pasted correctly with either
the minimal buggy zshrc or the current one I'm using. Much gratitude for
the work!

Best Regards,

Yen Chi Hsuan

On Fri, Oct 30, 2015 at 11:02 PM, Daniel Shahaf <d.s@daniel.shahaf.name>
wrote:

> Peter Stephenson wrote on Thu, Oct 29, 2015 at 16:56:25 +0000:
> > On Fri, 30 Oct 2015 01:25:49 +0900
> > Jun T. <takimoto-j@kba.biglobe.ne.jp> wrote:
> > > E01options.ztst also need be updated; otherwise it fails as follows:
> > >
> > > *** /tmp/zsh.ztst.err.65239 Fri Oct 30 00:54:52 2015
> > > --- /tmp/zsh.ztst.terr.65239        Fri Oct 30 00:54:52 2015
> > > ***************
> > > *** 1,3 ****
> > > --- 1,4 ----
> > >   fn:3: scalar parameter foo1 created globally in function
> > >   fn:5: scalar parameter foo1 created globally in function
> > >   fn:15: math parameter foo5 created globally in function fn
> > > + fn:15: numeric parameter foo5 created globally in function
> >
> > Hmmm... I must have been asleep when the math test was added, since
> > there's no reason at all why it should be inconsistent with the others.
> > Either report the function or not.
> >
> > -?fn:3: scalar parameter foo1 created globally in function
> > -?fn:5: scalar parameter foo1 created globally in function
> > -?fn:15: math parameter foo5 created globally in function fn
> > +?fn:3: scalar parameter foo1 created globally in function fn
> > +?fn:5: scalar parameter foo1 created globally in function fn
> > +?fn:15: numeric parameter parameter foo5 created globally in function fn
>
> Thanks for improving it ☺
>
> I'll fix the "numeric parameter parameter" thing.
>
> > This ports the code to report the function.  The math-specific code is
> > no longer needed now we have a check in setnparam().  I've been
> > overcautious since I can't see why we'd fail to find a function
> > on the trace stack if locallevel is non-zero.
>
> I can't see when that would happen, either; the reason for the NULL
> checks might be, more than anything else, that I wasn't familiar with
> the C funcstack data structure.
>
> Thanks,
>
> Daniel
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-11-03 16:31 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-15 13:54 Bug: bracketed-paste-magic + ztcp causes wrong pasted contents for CJK payloads Chi Hsuan Yen
2015-10-15 20:54 ` Bart Schaefer
2015-10-16  0:25   ` Bart Schaefer
2015-10-16 19:40     ` Chi Hsuan Yen
2015-10-18 15:52       ` Bart Schaefer
2015-10-18 16:19         ` Chi Hsuan Yen
2015-10-28  2:43     ` Bart Schaefer
2015-10-28  3:23       ` Bart Schaefer
2015-10-28  9:35         ` Peter Stephenson
2015-10-28 17:07           ` Bart Schaefer
2015-10-28 17:44             ` Peter Stephenson
2015-10-28 23:38               ` Bart Schaefer
2015-10-29  9:31                 ` Peter Stephenson
2015-10-29 14:51                   ` Peter Stephenson
2015-10-29 15:00                     ` Bart Schaefer
2015-10-29 15:10                       ` Peter Stephenson
2015-10-29 16:25                     ` Jun T.
2015-10-29 16:56                       ` Peter Stephenson
2015-10-30 15:02                         ` Daniel Shahaf
2015-11-03 16:31                           ` Chi Hsuan Yen

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).