mksh 49-1 no longer handles Windows line endings

mksh 49-1 no longer handles Windows line endings

[Ryan Dortmans]

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

Hi,

The latest version of mksh no longer handles Windows line endings in
ksh scripts when in a text-mode file system.

To reproduce:

$ mount -o text c:/textmode /textmode
(or use an existing text-mode mount)

$ echo -e "\necho \"test\"\n" > /textmode/test.ksh
$ ksh /textmode/test.ksh
: not foundsh[1]:
test
: not foundsh[3]:

$ d2u /textmode/test.ksh
dos2unix: converting file /textmode/test.ksh to Unix format...

$ ksh /textmode/test.ksh
test

Reverting to 48b-1 and retesting gives the correct behaviour
regardless of whether it has dos or unix line endings.


I have investigated the differences in the source code of the two
versions and found that 49-1 added O_BINARY flag to all of the open
calls. I took these flags out and was able to confirm that it fixes
the issue. I have attached two patches that do the same thing. One
removes the flag, the other commented out the #ifdef O_BINARY line in
the header so that #define O_BINARY 0 is always defined. These aren't
intended as actual patches to mksh, merely examples of how to fix the
issue.

The mksh changelog (https://www.mirbsd.org/mksh.htm#clog), indicates
why this change was made:
[tg] Add O_BINARY to all open(2) calls for OS/2 kLIBC support

The website also indicates that "No workarounds for .exe suffixes or
other platform-specific quirks have been or will be added." So I doubt
I could get this fixed upstream. Could the maintainer please take a
look at this?

Cheers,

Ryan Dortmans

[-- Attachment #2: mksh491_fix1.patch --]
[-- Type: application/octet-stream, Size: 4118 bytes --]

diff -rupN mksh491/mksh/exec.c mksh491_fix1a/mksh/exec.c
--- mksh491/mksh/exec.c	2014-01-12 03:26:52.000000000 +1100
+++ mksh491_fix1a/mksh/exec.c	2014-12-05 10:18:53.362891300 +1100
@@ -865,7 +865,7 @@ scriptexec(struct op *tp, const char **a
 	*tp->args-- = tp->str;
 
 #ifndef MKSH_SMALL
-	if ((fd = open(tp->str, O_RDONLY | O_BINARY)) >= 0) {
+	if ((fd = open(tp->str, O_RDONLY)) >= 0) {
 		/* read first MAXINTERP octets from file */
 		if (read(fd, buf, sizeof(buf)) <= 0)
 			/* read error -> no good */
@@ -1370,7 +1370,7 @@ iosetup(struct ioword *iop, struct tbl *
 			warningf(true, "%s: %s", cp, "restricted");
 			return (-1);
 		}
-		u = open(cp, flags | O_BINARY, 0666);
+		u = open(cp, flags, 0666);
 	}
 	if (u < 0) {
 		/* herein() may already have printed message */
@@ -1503,7 +1503,7 @@ herein(struct ioword *iop, char **resbuf
 	 * so temp doesn't get removed too soon).
 	 */
 	h = maketemp(ATEMP, TT_HEREDOC_EXP, &e->temps);
-	if (!(shf = h->shf) || (fd = open(h->tffn, O_RDONLY | O_BINARY, 0)) < 0) {
+	if (!(shf = h->shf) || (fd = open(h->tffn, O_RDONLY, 0)) < 0) {
 		i = errno;
 		warningf(true, "can't %s temporary file %s: %s",
 		    !shf ? "create" : "open", h->tffn, cstrerror(i));
diff -rupN mksh491/mksh/funcs.c mksh491_fix1a/mksh/funcs.c
--- mksh491/mksh/funcs.c	2014-01-06 06:20:56.000000000 +1100
+++ mksh491_fix1a/mksh/funcs.c	2014-12-05 10:18:22.433122200 +1100
@@ -3610,7 +3610,7 @@ c_cat(const char **wp)
 			fn = *wp++;
 			if (fn[0] == '-' && fn[1] == '\0')
 				fd = STDIN_FILENO;
-			else if ((fd = open(fn, O_RDONLY | O_BINARY)) < 0) {
+			else if ((fd = open(fn, O_RDONLY)) < 0) {
 				eno = errno;
 				bi_errorf("%s: %s", fn, cstrerror(eno));
 				rv = 1;
diff -rupN mksh491/mksh/histrap.c mksh491_fix1a/mksh/histrap.c
--- mksh491/mksh/histrap.c	2013-10-09 22:59:53.000000000 +1100
+++ mksh491_fix1a/mksh/histrap.c	2014-12-05 10:19:38.243458300 +1100
@@ -720,8 +720,7 @@ hist_init(Source *s)
 
  retry:
 	/* we have a file and are interactive */
-	if ((fd = open(hname, O_RDWR | O_CREAT | O_APPEND | O_BINARY,
-	    0600)) < 0)
+	if ((fd = open(hname, O_RDWR | O_CREAT | O_APPEND, 0600)) < 0)
 		return;
 
 	histfd = savefd(fd);
@@ -757,7 +756,7 @@ hist_init(Source *s)
 			/* create temporary file */
 			nhname = shf_smprintf("%s.%d", hname, (int)procpid);
 			if ((fd = open(nhname, O_RDWR | O_CREAT | O_TRUNC |
-			    O_EXCL | O_BINARY, 0600)) < 0) {
+			    O_EXCL, 0600)) < 0) {
 				/* just don't truncate then, meh. */
 				goto hist_trunc_dont;
 			}
diff -rupN mksh491/mksh/main.c mksh491_fix1a/mksh/main.c
--- mksh491/mksh/main.c	2014-01-12 05:10:05.000000000 +1100
+++ mksh491_fix1a/mksh/main.c	2014-12-05 10:20:05.658026300 +1100
@@ -1644,8 +1644,7 @@ maketemp(Area *ap, Temp_type type, struc
 	} while (len < 5);
 
 	/* cyclically attempt to open a temporary file */
-	while ((i = open(tp->tffn, O_CREAT | O_EXCL | O_RDWR | O_BINARY,
-	    0600)) < 0) {
+	while ((i = open(tp->tffn, O_CREAT | O_EXCL | O_RDWR, 0600)) < 0) {
 		if (errno != EEXIST)
 			goto maketemp_out;
 		/* count down from z to a then from 9 to 0 */
diff -rupN mksh491/mksh/misc.c mksh491_fix1a/mksh/misc.c
--- mksh491/mksh/misc.c	2014-01-06 08:57:52.000000000 +1100
+++ mksh491_fix1a/mksh/misc.c	2014-12-05 10:20:35.048707400 +1100
@@ -1996,9 +1996,9 @@ chvt(const Getopt *go)
 #endif
 	    }
 	}
-	if ((fd = open(dv, O_RDWR | O_BINARY)) < 0) {
+	if ((fd = open(dv, O_RDWR)) < 0) {
 		sleep(1);
-		if ((fd = open(dv, O_RDWR | O_BINARY)) < 0) {
+		if ((fd = open(dv, O_RDWR)) < 0) {
 			errorf("%s: %s %s", "chvt", "can't open", dv);
 		}
 	}
diff -rupN mksh491/mksh/shf.c mksh491_fix1a/mksh/shf.c
--- mksh491/mksh/shf.c	2013-10-09 22:59:55.000000000 +1100
+++ mksh491_fix1a/mksh/shf.c	2014-12-05 10:21:11.628799600 +1100
@@ -62,7 +62,7 @@ shf_open(const char *name, int oflags, i
 	shf->flags = SHF_ALLOCS;
 	/* Rest filled in by reopen. */
 
-	fd = open(name, oflags | O_BINARY, mode);
+	fd = open(name, oflags, mode);
 	if (fd < 0) {
 		eno = errno;
 		afree(shf, shf->areap);

[-- Attachment #3: mksh491_fix2.patch --]
[-- Type: application/octet-stream, Size: 382 bytes --]

diff -rupN mksh491/mksh/sh.h mksh491_fix2a/mksh/sh.h
--- mksh491/mksh/sh.h	2014-01-12 05:10:07.000000000 +1100
+++ mksh491_fix2a/mksh/sh.h	2014-12-05 11:12:08.587647700 +1100
@@ -401,9 +401,9 @@ extern int __cdecl setegid(gid_t);
 #endif
 #endif
 
-#ifndef O_BINARY
+//#ifndef O_BINARY
 #define O_BINARY	0
-#endif
+//#endif
 
 #ifdef MKSH__NO_SYMLINK
 #undef S_ISLNK

[-- Attachment #4: Type: text/plain, Size: 218 bytes --]

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: mksh 49-1 no longer handles Windows line endings

[Warren Young]

On Dec 10, 2014, at 6:20 PM, Ryan Dortmans <ryandort.cygwin@gmail.com> wrote:

> The latest version of mksh no longer handles Windows line endings

[snip]

> <mksh491_fix1.patch><mksh491_fix2.patch>—

mksh is currently up for adoption:

  http://cygwin.1069669.n5.nabble.com/Packages-up-for-adoption-td111353.html

I briefly considered adopting it, mainly for historical purposes.[1]  But, you seem to have a much stronger itch here.  You should adopt it.

Actually, I think whoever adopts mksh should replace it with the real Korn shell.  The whole pdksh -> mksh code path reflects the historical unavailability of the true ksh source code, but that changed in 2000.[2]


[1] It’s sometimes useful to be able to compare ksh88/93 behavior to Bash and Zsh.

[2] http://kornshell.com/
--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: mksh 49-1 no longer handles Windows line endings

[Eric Blake]

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

On 12/10/2014 08:21 PM, Warren Young wrote:
> On Dec 10, 2014, at 6:20 PM, Ryan Dortmans <ryandort.cygwin@gmail.com> wrote:
> 
>> The latest version of mksh no longer handles Windows line endings
> 
> [snip]
> 
>> <mksh491_fix1.patch><mksh491_fix2.patch>—
> 
> mksh is currently up for adoption:
> 
>   http://cygwin.1069669.n5.nabble.com/Packages-up-for-adoption-td111353.html
> 
> I briefly considered adopting it, mainly for historical purposes.[1]  But, you seem to have a much stronger itch here.  You should adopt it.
> 
> Actually, I think whoever adopts mksh should replace it with the real Korn shell.  The whole pdksh -> mksh code path reflects the historical unavailability of the true ksh source code, but that changed in 2000.[2]
> 
> 
> [1] It’s sometimes useful to be able to compare ksh88/93 behavior to Bash and Zsh.

Actually, I think it is good to have BOTH mksh (BSD derivative) AND
ksh93 (Korn shell).  It is useful to compare all of the shells, not just
a subset of them.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]