Re: cat as a builtin command

Re: cat as a builtin command

[Stephane Chazelas]

2014-08-29 14:03:43 +0100, Peter Stephenson:
[...]
> mycat() {
>   emulate -L zsh
>   unsetopt multibyte
>   zmodload zsh/mapfile
>   print -r $mapfile[$1]
> }
[...]

That should be:

print -rn -- $mapfile[$1]

What would be the possible effects of *not* turning off
"multibyte" here?

-- 
Stephane

Re: cat as a builtin command

[Peter Stephenson]

On Fri, 29 Aug 2014 21:15:57 +0100
Stephane Chazelas <stephane.chazelas@gmail.com> wrote:
> 2014-08-29 14:03:43 +0100, Peter Stephenson:
> [...]
> > mycat() {
> >   emulate -L zsh
> >   unsetopt multibyte
> >   zmodload zsh/mapfile
> >   print -r $mapfile[$1]
> > }
> [...]
> 
> That should be:
> 
> print -rn -- $mapfile[$1]
> 
> What would be the possible effects of *not* turning off
> "multibyte" here?

Logically if you have an 8-bit character set defined then the shell
expects command lines to use it and you're relying on the shell
shrugging its shoulders and passing on raw octets anyway, which is
implemented but a bit hit and miss.  However, with just a raw print, and
print expansions turned off, I would hope it works for any input, since
there's not a lot left to go wrong in that case.  It would need scanning
the command argument handling and print code in some detail and with
interestinf failure cases in mind (e.g. partial characters) to be sure.

pws

"does not contain a file descriptor" (Re: cat as a builtin command)

[Bart Schaefer]

On Sep 1, 11:39am, Bart Schaefer wrote:
}
} torch% exec {fd}<&-
} zsh: parameter fd does not contain a file descriptor
} 
} It appears that only an UNSET parameter name triggers the "does not
} contain" error; a set-but-empty parameter is treated as 0 and closes
} standard input

Do we really want that behavior?  Or the following behavior?

torch% declare fd="12+7"
torch% exec {fd}<&-
zsh: failed to close file descriptor 19: bad file descriptor

Patch below makes this somewhat more rigorous; but if the parameter is
declared as an integer, then even its string value is zero when no value
has been assigned, so the original problem can't be completely avoided.
(As far as I can tell, perhaps I'm wrong?)

I won't commit this without getting feedback, because it doesn't work
correctly with parameters declared as integers in bases other than 10
(also 8 if you set OCTAL_ZEROS, and also 16 if you set C_BASES).  So
other suggestions are welcome ... or we can just document that all file
descriptors must be base-10 integers, in which case we should change the
base from 0 to 10 in the zstrtol() call.


diff --git a/Src/exec.c b/Src/exec.c
index bf50d0f..7fb51bc 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3060,8 +3060,9 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		    } else if (v->pm->node.flags & PM_READONLY) {
 			bad = 2;
 		    } else {
-			fn->fd1 = (int)getintvalue(v);
-			if (errflag)
+			char *s = getstrvalue(v), *t;
+			fn->fd1 = zstrtol(s, &t, 0);
+			if (errflag || s == t || *t)
 			    bad = 1;
 			else if (fn->fd1 <= max_zsh_fd) {
 			    if (fn->fd1 >= 10 &&

Re: "does not contain a file descriptor" (Re: cat as a builtin command)

[Peter Stephenson]

On Mon, 01 Sep 2014 12:44:03 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:

> On Sep 1, 11:39am, Bart Schaefer wrote:
> }
> } torch% exec {fd}<&-
> } zsh: parameter fd does not contain a file descriptor
> } 
> } It appears that only an UNSET parameter name triggers the "does not
> } contain" error; a set-but-empty parameter is treated as 0 and closes
> } standard input
> 
> Do we really want that behavior?

No, that doesn't seem a good idea.

> Or the following behavior?
> 
> torch% declare fd="12+7"
> torch% exec {fd}<&-
> zsh: failed to close file descriptor 19: bad file descriptor
> 
> Patch below makes this somewhat more rigorous; but if the parameter is
> declared as an integer, then even its string value is zero when no value
> has been assigned, so the original problem can't be completely avoided.
> (As far as I can tell, perhaps I'm wrong?)

The default value for an integer is 0, unless you explicitly set it.
There's no separate string default --- the string value comes from
converting the integer value each time it's needed.  So I think this is
unavoidable.  We could advise users to initialise fd variables to -1.

> I won't commit this without getting feedback, because it doesn't work
> correctly with parameters declared as integers in bases other than 10
> (also 8 if you set OCTAL_ZEROS, and also 16 if you set C_BASES).  So
> other suggestions are welcome ... or we can just document that all file
> descriptors must be base-10 integers, in which case we should change the
> base from 0 to 10 in the zstrtol() call.

I don't think this problem's fatal, but it's surprising if there isn't
some way or other of getting this to work by indicating the base
appropriately.

pws

Re: "does not contain a file descriptor" (Re: cat as a builtin command)

[Bart Schaefer]

On Sep 2,  9:43am, Peter Stephenson wrote:
}
} On Mon, 01 Sep 2014 12:44:03 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} 
} > I won't commit this without getting feedback, because it doesn't work
} > correctly with parameters declared as integers in bases other than 10
} 
} I don't think this problem's fatal, but it's surprising if there isn't
} some way or other of getting this to work by indicating the base
} appropriately.

Hm.  How about this, then?

diff --git a/Src/exec.c b/Src/exec.c
index bf50d0f..fb2739a 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3050,7 +3050,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		break;
 	    case REDIR_CLOSE:
 		if (fn->varid) {
-		    char *s = fn->varid;
+		    char *s = fn->varid, *t;
 		    struct value vbuf;
 		    Value v;
 		    int bad = 0;
@@ -3060,13 +3060,25 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		    } else if (v->pm->node.flags & PM_READONLY) {
 			bad = 2;
 		    } else {
-			fn->fd1 = (int)getintvalue(v);
+			s = getstrvalue(v);
 			if (errflag)
 			    bad = 1;
-			else if (fn->fd1 <= max_zsh_fd) {
-			    if (fn->fd1 >= 10 &&
-				 fdtable[fn->fd1] == FDT_INTERNAL)
-				bad = 3;
+			else {
+			    fn->fd1 = zstrtol(s, &t, 0);
+			    if (s == t)
+				bad = 1;
+			    else if (*t) {
+				/* Check for base#number format */
+				if (*t == '#' && *s != '0')
+				    fn->fd1 = zstrtol(s = t+1, &t, fn->fd1);
+				if (s == t || *t)
+				    bad = 1;
+			    }
+			    if (!bad && fn->fd1 <= max_zsh_fd) {
+				if (fn->fd1 >= 10 &&
+				    fdtable[fn->fd1] == FDT_INTERNAL)
+				    bad = 3;
+			    }
 			}
 		    }
 		    if (bad) {

-- 
Barton E. Schaefer

Re: "does not contain a file descriptor" (Re: cat as a builtin command)

[Peter Stephenson]

On Wed, 03 Sep 2014 00:15:21 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Sep 2,  9:43am, Peter Stephenson wrote:
> }
> } On Mon, 01 Sep 2014 12:44:03 -0700
> } Bart Schaefer <schaefer@brasslantern.com> wrote:
> } 
> } > I won't commit this without getting feedback, because it doesn't work
> } > correctly with parameters declared as integers in bases other than 10
> } 
> } I don't think this problem's fatal, but it's surprising if there isn't
> } some way or other of getting this to work by indicating the base
> } appropriately.
> 
> Hm.  How about this, then?

Yes, that's sort of what I was thinking.  Rather minor but if it just
works we don't have to document our way round it, have people not read
the documentation, etc.

Thanks.
pws