PATCH: fix for autoloading and compiling under Cygwin

PATCH: fix for autoloading and compiling under Cygwin

[JohnW]

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

I was actually overlooking writing of zwc files in my first fix. Here's a
new patch that really fixes the problem, with a test case, too. I've done
two things here:

* I added the O_BINARY flag to all calls to 'open' on zwc files.
* For text files, I changed the code to treat the result of 'seek' as an
upper bound on the length of the file. For the actual length, the return
value of 'read' is used. Checks that 'seek' and 'read' return the same thing
have been changed to check that 'seek' and 'read' both return nonnegative
values.

-----Original Message-----
From: Borsenkow Andrej [mailto:Andrej.Borsenkow@mow.siemens.ru]
Sent: Monday, December 17, 2001 8:43 AM
To: 'Zsh hackers list'
Cc: JohnW@bops.com
Subject: RE: forward: fix for problem under cygwin




> 
> I found a Unixism in zsh that prevents autoloading and function
compilation
> from working under cygwin. There are several bits of code that assume
> character offsets are the same as byte offsets in files. I've fixed
this by
> either opening files in binary mode or relaxing the error checking.

No. We must never assume files are DOS text files because it makes them
non-portable not only between Cygwin/Unix but even between two different
Cygwin instances (just think about remounting the same directory in
binary mode after creating file in text mode).

We must ensure that read/write of zwc files always happens in binary
mode. This will make it possible to share files between Cygwin/Unix as
well. Your patch has several O_BINARY for reading but I do not see
O_BINARY for writing.

Of course, after doing it one probably has to recreate zwc files to be
sure.

-andrej


[-- Attachment #2: zsh-patch.diff --]
[-- Type: application/octet-stream, Size: 4364 bytes --]

Only in zsh-4.0.4-patched/Config: defs.mk
Only in zsh-4.0.4-patched/Doc: Makefile
Only in zsh-4.0.4/Etc: .zsh-development-guide.swp
Only in zsh-4.0.4-patched/Etc: Makefile
Only in zsh-4.0.4-patched: Makefile
Only in zsh-4.0.4-patched/Src: .parse.c.swp
Only in zsh-4.0.4-patched/Src/Builtins: Makefile
Only in zsh-4.0.4-patched/Src/Builtins: Makefile.in
Only in zsh-4.0.4-patched/Src: Makefile
Only in zsh-4.0.4-patched/Src: Makemod
Only in zsh-4.0.4-patched/Src: Makemod.in
Only in zsh-4.0.4-patched/Src/Modules: Makefile
Only in zsh-4.0.4-patched/Src/Modules: Makefile.in
Only in zsh-4.0.4-patched/Src/Modules: cap.exp
Only in zsh-4.0.4-patched/Src/Zle: Makefile
Only in zsh-4.0.4-patched/Src/Zle: Makefile.in
diff -ur zsh-4.0.4/Src/exec.c zsh-4.0.4-patched/Src/exec.c
--- zsh-4.0.4/Src/exec.c	Wed Oct 24 06:16:32 2001
+++ zsh-4.0.4-patched/Src/exec.c	Mon Dec 10 13:33:48 2001
@@ -3470,7 +3470,7 @@
 getfpfunc(char *s, int *ksh)
 {
     char **pp, buf[PATH_MAX];
-    off_t len;
+    off_t len, textlen;
     char *d;
     Eprog r;
     int fd;
@@ -3490,12 +3490,12 @@
 	    if ((len = lseek(fd, 0, 2)) != -1) {
 		d = (char *) zalloc(len + 1);
 		lseek(fd, 0, 0);
-		if (read(fd, d, len) == len) {
+		if ((textlen = read(fd, d, len)) >= 0) {
 		    char *oldscriptname = scriptname;
 
 		    close(fd);
-		    d[len] = '\0';
-		    d = metafy(d, len, META_REALLOC);
+		    d[textlen] = '\0';
+		    d = metafy(d, textlen, META_REALLOC);
 
 		    scriptname = dupstring(s);
 		    r = parse_string(d, 1);
diff -ur zsh-4.0.4/Src/parse.c zsh-4.0.4-patched/Src/parse.c
--- zsh-4.0.4/Src/parse.c	Wed Oct 24 06:16:32 2001
+++ zsh-4.0.4-patched/Src/parse.c	Mon Dec 17 12:30:00 2001
@@ -2405,7 +2405,7 @@
     int fd, v = 0;
     wordcode buf[FD_PRELEN + 1];
 
-    if ((fd = open(name, O_RDONLY)) < 0) {
+    if ((fd = open(name, O_RDONLY | O_BINARY)) < 0) {
 	if (err)
 	    zwarnnam(nam, "can't open zwc file: %s", name, 0);
 	return NULL;
@@ -2542,7 +2542,7 @@
 static int
 build_dump(char *nam, char *dump, char **files, int ali, int map, int flags)
 {
-    int dfd, fd, hlen, tlen, flen, ona = noaliases;
+    int dfd, fd, hlen, tlen, flen, ona = noaliases, textlen;
     LinkList progs;
     char *file;
     Eprog prog;
@@ -2551,7 +2551,7 @@
     if (!strsfx(FD_EXT, dump))
 	dump = dyncat(dump, FD_EXT);
 
-    if ((dfd = open(dump, O_WRONLY|O_CREAT, 0600)) < 0) {
+    if ((dfd = open(dump, O_WRONLY|O_CREAT|O_BINARY, 0600)) < 0) {
 	zwarnnam(nam, "can't write zwc file: %s", dump, 0);
 	return 1;
     }
@@ -2577,9 +2577,8 @@
 	    return 1;
 	}
 	file = (char *) zalloc(flen + 1);
-	file[flen] = '\0';
 	lseek(fd, 0, 0);
-	if (read(fd, file, flen) != flen) {
+	if ((textlen = read(fd, file, flen)) < 0) {
 	    close(fd);
 	    close(dfd);
 	    zfree(file, flen);
@@ -2589,7 +2588,8 @@
 	    return 1;
 	}
 	close(fd);
-	file = metafy(file, flen, META_REALLOC);
+	file[textlen] = '\0';
+	file = metafy(file, textlen, META_REALLOC);
 
 	if (!(prog = parse_string(file, 1)) || errflag) {
 	    errflag = 0;
@@ -2682,7 +2682,7 @@
     if (!strsfx(FD_EXT, dump))
 	dump = dyncat(dump, FD_EXT);
 
-    if ((dfd = open(dump, O_WRONLY|O_CREAT, 0600)) < 0) {
+    if ((dfd = open(dump, O_WRONLY|O_CREAT|O_BINARY, 0600)) < 0) {
 	zwarnnam(nam, "can't write zwc file: %s", dump, 0);
 	return 1;
     }
@@ -2814,7 +2814,7 @@
 	off = 0;
         mlen = len;
     }
-    if ((fd = open(dump, O_RDONLY)) < 0)
+    if ((fd = open(dump, O_RDONLY | O_BINARY)) < 0)
 	return;
 
     fd = movefd(fd);
@@ -3014,7 +3014,7 @@
 	    Patprog *pp;
 	    int np, fd, po = h->npats * sizeof(Patprog);
 
-	    if ((fd = open(file, O_RDONLY)) < 0 ||
+	    if ((fd = open(file, O_RDONLY | O_BINARY)) < 0 ||
 		lseek(fd, ((h->start * sizeof(wordcode)) +
 			   ((fdflags(d) & FDF_OTHER) ? fdother(d) : 0)), 0) < 0) {
 		if (fd >= 0)
Only in zsh-4.0.4-patched/Test: Makefile
Only in zsh-4.0.4-patched: config.cache
Only in zsh-4.0.4-patched: config.h
Only in zsh-4.0.4-patched: config.log
Only in zsh-4.0.4-patched: config.modules
Only in zsh-4.0.4-patched: config.status
Only in zsh-4.0.4-patched: conftest1.c
Only in zsh-4.0.4-patched: conftest1.o
Only in zsh-4.0.4-patched: conftest2.c
Only in zsh-4.0.4-patched: stamp-h
Only in zsh-4.0.4-patched: tags

[-- Attachment #3: A06autoload.ztst --]
[-- Type: application/octet-stream, Size: 951 bytes --]

%prep

 mkdir func.tmp

 echo 'echo zinit; function zfunc() { echo zeval }' >func.tmp/zfunc
 
 echo 'echo kinit; function kfunc() { echo keval }' >func.tmp/kfunc
 
 echo 'echo eval' >func.tmp/script
 
 echo 'echo cinit; compiled() { echo ceval };' >func.tmp/compiled

 fpath=(func.tmp)

%test
 
 autoload +X -z zfunc script
0q:zsh-style autoloading

 autoload +X -k kfunc
0q:ksh-style autoloading

 zfunc
0q:Only initialization code is executed.
>zinit

 zfunc
0q:Only the function definition is executed.
>zeval

 kfunc
0q:The function is initialized and called.
>kinit
>keval

 kfunc
0q:Only the function definition is executed.
>keval

 script
0q:The whole file is executed.
>eval

 zcompile func.tmp/compiled
0q:A file can be compiled.
 
 autoload -w func.tmp/compiled
0q:A compiled file can be autoloaded.

 compiled
0q:A function from a compiled file can be called.
>cinit

 compiled
0q:A compiled function is not re-initialized.
>ceval

%clean

RE: PATCH: fix for autoloading and compiling under Cygwin

[Borsenkow Andrej]

> 
> I was actually overlooking writing of zwc files in my first fix.
Here's a
> new patch that really fixes the problem, with a test case, too. I've
done
> two things here:
> 
> * I added the O_BINARY flag to all calls to 'open' on zwc files.

That is needed in any case and is not harmful at all. Any system that
does not support this flag? Should probably add

#ifndef O_BINARY
#define O_BINARY 0
#endif

somewhere in ... zsh.h?

> * For text files, I changed the code to treat the result of 'seek' as
an
> upper bound on the length of the file. For the actual length, the
return
> value of 'read' is used. Checks that 'seek' and 'read' return the same
thing
> have been changed to check that 'seek' and 'read' both return
nonnegative
> values.
> 

I have mixed feelings about it. It needed to be under #ifdef __CYGWIN__
in the first place. It also defeats error checking completely.

Have you looked how bash on Cygwin does it?

-andrej

Re: PATCH: fix for autoloading and compiling under Cygwin

[Peter Stephenson]

Borsenkow Andrej wrote:
> > * I added the O_BINARY flag to all calls to 'open' on zwc files.
> 
> That is needed in any case and is not harmful at all. Any system that
> does not support this flag? Should probably add
> 
> #ifndef O_BINARY
> #define O_BINARY 0
> #endif
> 
> somewhere in ... zsh.h?

I was about to say of course it's always been there --- but it's missing on
Solaris.  I'm thinking of the "b" flag to fopen, which has always been
there.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

RE: PATCH: fix for autoloading and compiling under Cygwin

[JohnW]

When bash reads whole files, it uses fstat to find out how much memory to
allocate to store the file and then takes the result of read to be the
actual length of the file. All the error checks on read just make sure the
result isn't negative, except one case that tries to read a single
character. That case makes sure read returns 1.

Regarding O_BINARY, bash does something similar to what you suggest. Here's
the actual code:

/* If we're compiling for __EMX__ (OS/2) or __CYGWIN__ (cygwin32 environment
   on win 95/98/nt), we want to open files with O_BINARY mode so that there
   is no \n -> \r\n conversion performed.  On other systems, we don't want
to
   mess around with O_BINARY at all, so we ensure that it's defined to 0. */
#if defined (__EMX__) || defined (__CYGWIN__)
#  ifndef O_BINARY
#    define O_BINARY 0
#  endif
#else /* !__EMX__ && !__CYGWIN__ */
#  undef O_BINARY
#  define O_BINARY 0
#endif /* !__EMX__ && !__CYGWIN__ */

-----Original Message-----
From: Borsenkow Andrej [mailto:Andrej.Borsenkow@mow.siemens.ru]
Sent: Tuesday, December 18, 2001 2:11 AM
To: JohnW@bops.com; zsh-workers@sunsite.dk
Subject: RE: PATCH: fix for autoloading and compiling under Cygwin



> 
> I was actually overlooking writing of zwc files in my first fix.
Here's a
> new patch that really fixes the problem, with a test case, too. I've
done
> two things here:
> 
> * I added the O_BINARY flag to all calls to 'open' on zwc files.

That is needed in any case and is not harmful at all. Any system that
does not support this flag? Should probably add

#ifndef O_BINARY
#define O_BINARY 0
#endif

somewhere in ... zsh.h?

> * For text files, I changed the code to treat the result of 'seek' as
an
> upper bound on the length of the file. For the actual length, the
return
> value of 'read' is used. Checks that 'seek' and 'read' return the same
thing
> have been changed to check that 'seek' and 'read' both return
nonnegative
> values.
> 

I have mixed feelings about it. It needed to be under #ifdef __CYGWIN__
in the first place. It also defeats error checking completely.

Have you looked how bash on Cygwin does it?

-andrej