CVS commit: [netbsd-8] src/bin/sh
Module Name:src Committed By: martin Date: Thu Oct 27 16:16:50 UTC 2022 Modified Files: src/bin/sh [netbsd-8]: miscbltin.c Log Message: Pull up following revision(s) (requested by kre in ticket #1779): bin/sh/miscbltin.c: revision 1.51 bin/sh/miscbltin.c: revision 1.52 PR bin/56972 Fix escape ('\') handling in sh read builtin. In 1.35 (March 2005) (the big read fixup), most escape handling and IFS processing in the read builtin was corrected. However 2 cases were missed, one is a word (something to be assigned to any variable but the last) in which every character is escaped (the code was relying on a non-escaped char to set the "in a word" status), and second trailing IFS whitespace at the end of the line was being deleted, even if the chars had been escaped (the escape chars are no longer present). See the PR for more details (including the case that detected the problem). After fixing this, I looked at the FreeBSD code (normally might do it before, but these fixes were trivial) to check their implementation. Their code does similar things to ours now does, but in a completely different way, their read builtin is more complex than ours needs to be (they handle more options). For anyone tempted to simply incorporate their code, note that it relies upon infrastructure changes elsewhere in the shell, so would not be a simple cut and drop in exercise. This needs pullups to -3 -4 -5 -6 -7 -8 and -9 (fortunately this is happening before -10 is branched, so will never be broken this way there). - Don't output the error for bad usage (no var name given) after already writing the prompt (set with the -p option). That results in nonsense like: $ read -p foo fooread: arg count While here, improve the error message so it means something. Now we will get: $ read -p foo read: variable name required Usage: read [-r] [-p prompt] var... [Detected by code reading while doing the work for the previous fix] To generate a diff of this commit: cvs rdiff -u -r1.44 -r1.44.2.1 src/bin/sh/miscbltin.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/bin/sh/miscbltin.c diff -u src/bin/sh/miscbltin.c:1.44 src/bin/sh/miscbltin.c:1.44.2.1 --- src/bin/sh/miscbltin.c:1.44 Sat May 13 15:03:34 2017 +++ src/bin/sh/miscbltin.c Thu Oct 27 16:16:50 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: miscbltin.c,v 1.44 2017/05/13 15:03:34 gson Exp $ */ +/* $NetBSD: miscbltin.c,v 1.44.2.1 2022/10/27 16:16:50 martin Exp $ */ /*- * Copyright (c) 1991, 1993 @@ -37,7 +37,7 @@ #if 0 static char sccsid[] = "@(#)miscbltin.c 8.4 (Berkeley) 5/4/95"; #else -__RCSID("$NetBSD: miscbltin.c,v 1.44 2017/05/13 15:03:34 gson Exp $"); +__RCSID("$NetBSD: miscbltin.c,v 1.44.2.1 2022/10/27 16:16:50 martin Exp $"); #endif #endif /* not lint */ @@ -99,6 +99,7 @@ readcmd(int argc, char **argv) int i; int is_ifs; int saveall = 0; + ptrdiff_t wordlen = 0; rflag = 0; prompt = NULL; @@ -109,14 +110,15 @@ readcmd(int argc, char **argv) rflag = 1; } + if (*(ap = argptr) == NULL) + error("variable name required\n" + "Usage: read [-r] [-p prompt] var..."); + if (prompt && isatty(0)) { out2str(prompt); flushall(); } - if (*(ap = argptr) == NULL) - error("arg count"); - if ((ifs = bltinlookup("IFS", 1)) == NULL) ifs = " \t\n"; @@ -136,7 +138,7 @@ readcmd(int argc, char **argv) break; } if (c != '\n') -STPUTC(c, p); +goto wdch; continue; } if (c == '\n') @@ -163,12 +165,14 @@ readcmd(int argc, char **argv) } if (is_ifs == 0) { + wdch:; /* append this character to the current variable */ startword = 0; if (saveall) /* Not just a spare terminator */ saveall++; STPUTC(c, p); + wordlen = p - stackblock(); continue; } @@ -186,11 +190,12 @@ readcmd(int argc, char **argv) setvar(*ap, stackblock(), 0); ap++; STARTSTACKSTR(p); + wordlen = 0; } STACKSTRNUL(p); /* Remove trailing IFS chars */ - for (; stackblock() <= --p; *p = 0) { + for (; stackblock() + wordlen <= --p; *p = 0) { if (!strchr(ifs, *p)) break; if (strchr(" \t\n", *p))
CVS commit: [netbsd-8] src/bin/sh
Module Name:src Committed By: martin Date: Thu Oct 27 16:16:50 UTC 2022 Modified Files: src/bin/sh [netbsd-8]: miscbltin.c Log Message: Pull up following revision(s) (requested by kre in ticket #1779): bin/sh/miscbltin.c: revision 1.51 bin/sh/miscbltin.c: revision 1.52 PR bin/56972 Fix escape ('\') handling in sh read builtin. In 1.35 (March 2005) (the big read fixup), most escape handling and IFS processing in the read builtin was corrected. However 2 cases were missed, one is a word (something to be assigned to any variable but the last) in which every character is escaped (the code was relying on a non-escaped char to set the "in a word" status), and second trailing IFS whitespace at the end of the line was being deleted, even if the chars had been escaped (the escape chars are no longer present). See the PR for more details (including the case that detected the problem). After fixing this, I looked at the FreeBSD code (normally might do it before, but these fixes were trivial) to check their implementation. Their code does similar things to ours now does, but in a completely different way, their read builtin is more complex than ours needs to be (they handle more options). For anyone tempted to simply incorporate their code, note that it relies upon infrastructure changes elsewhere in the shell, so would not be a simple cut and drop in exercise. This needs pullups to -3 -4 -5 -6 -7 -8 and -9 (fortunately this is happening before -10 is branched, so will never be broken this way there). - Don't output the error for bad usage (no var name given) after already writing the prompt (set with the -p option). That results in nonsense like: $ read -p foo fooread: arg count While here, improve the error message so it means something. Now we will get: $ read -p foo read: variable name required Usage: read [-r] [-p prompt] var... [Detected by code reading while doing the work for the previous fix] To generate a diff of this commit: cvs rdiff -u -r1.44 -r1.44.2.1 src/bin/sh/miscbltin.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: [netbsd-8] src/bin/sh
Module Name:src Committed By: martin Date: Thu Feb 24 10:07:47 UTC 2022 Modified Files: src/bin/sh [netbsd-8]: histedit.c Log Message: Pull up following revision(s) (requested by kre in ticket #1736): bin/sh/histedit.c: revision 1.60 After (a few days short of) 21 years, revert 1.25, which did nothing except make the -e option to "fc" fail to work (the commit message was about some other changes entirely, so I an only assume this was committed by mistake). It says a lot about the use of the fc command that no-one noticed that this did not work properly for all this time. Internally in sh, it is possible for built in commands to use either getopt(3) (from libc) or the much simpler internal shell nextopt() routine for option (flag) parsing.However it makes no sense to use getopt() and then access a global variable set only by nextopt() instead of the one getopt() sets (which is what the code had used previously, forever). Use the correct variable again. XXX pullup -9 -8 (-7 -6 -5 ...) To generate a diff of this commit: cvs rdiff -u -r1.48.8.2 -r1.48.8.3 src/bin/sh/histedit.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: [netbsd-8] src/bin/sh
Module Name:src Committed By: martin Date: Thu Feb 24 10:07:47 UTC 2022 Modified Files: src/bin/sh [netbsd-8]: histedit.c Log Message: Pull up following revision(s) (requested by kre in ticket #1736): bin/sh/histedit.c: revision 1.60 After (a few days short of) 21 years, revert 1.25, which did nothing except make the -e option to "fc" fail to work (the commit message was about some other changes entirely, so I an only assume this was committed by mistake). It says a lot about the use of the fc command that no-one noticed that this did not work properly for all this time. Internally in sh, it is possible for built in commands to use either getopt(3) (from libc) or the much simpler internal shell nextopt() routine for option (flag) parsing.However it makes no sense to use getopt() and then access a global variable set only by nextopt() instead of the one getopt() sets (which is what the code had used previously, forever). Use the correct variable again. XXX pullup -9 -8 (-7 -6 -5 ...) To generate a diff of this commit: cvs rdiff -u -r1.48.8.2 -r1.48.8.3 src/bin/sh/histedit.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/bin/sh/histedit.c diff -u src/bin/sh/histedit.c:1.48.8.2 src/bin/sh/histedit.c:1.48.8.3 --- src/bin/sh/histedit.c:1.48.8.2 Sat Aug 25 14:45:37 2018 +++ src/bin/sh/histedit.c Thu Feb 24 10:07:46 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: histedit.c,v 1.48.8.2 2018/08/25 14:45:37 martin Exp $ */ +/* $NetBSD: histedit.c,v 1.48.8.3 2022/02/24 10:07:46 martin Exp $ */ /*- * Copyright (c) 1993 @@ -37,7 +37,7 @@ #if 0 static char sccsid[] = "@(#)histedit.c 8.2 (Berkeley) 5/4/95"; #else -__RCSID("$NetBSD: histedit.c,v 1.48.8.2 2018/08/25 14:45:37 martin Exp $"); +__RCSID("$NetBSD: histedit.c,v 1.48.8.3 2022/02/24 10:07:46 martin Exp $"); #endif #endif /* not lint */ @@ -283,7 +283,7 @@ histcmd(volatile int argc, char ** volat (ch = getopt(argc, argv, ":e:lnrs")) != -1) switch ((char)ch) { case 'e': - editor = optionarg; + editor = optarg; break; case 'l': lflg = 1;