bugfix stdin heredoc (4bb42a30) · Commits · jan.koester / fresh

commands/shell/case.cpp

+33 −1

Changes for commands/shell/case.cpp: 33 added lines, 1 removed line.
Original line number	Diff line number	Diff line
		@@ -34,7 +34,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

		#include <iostream>

		#include <fcntl.h>
		#include <fnmatch.h>
		#include <unistd.h>

		#include "shell.h"
		#include "envoirment.h"
		@@ -148,6 +150,30 @@ namespace fresh { namespace cmd {
		int status=shell.running() ? 0 : shell.exitCode();
		bool matched=false;

		std::string trailing=shellWordsRawText(remainder,words,esacIdx+1,words.size());

		// "esac <<WORD" -- a here-doc redirect on the whole case
		// construct (the matched clause's own body may contain a "read"
		// meant to consume it), not more statements after it (this
		// builtin's other, "; more..." use of `trailing`, below). See
		// while.cpp's matching comment for the full story -- must be
		// applied before the clause-matching loop below runs, same
		// reasoning as while/for/if.
		int savedStdin=-1;
		std::string heredocTempFile;
		HeredocSpec heredocSpec;
		if(extractHeredocSpec(trailing,heredocSpec)){
		heredocTempFile=shell.collectHeredocBody(heredocSpec);
		if(!heredocTempFile.empty()){
		int fd=open(heredocTempFile.c_str(),O_RDONLY);
		if(fd>=0){
		savedStdin=dup(0);
		dup2(fd,0);
		close(fd);
		}
		}
		}

		for(size_t c=0;c<clauses.size() && !matched && shell.running();++c){
		std::string clauseText=shellWordsRawText(remainder,words,clauses[c].first,clauses[c].second);

		@@ -204,7 +230,13 @@ namespace fresh { namespace cmd {
		status=shell.parse(body);
		}

		std::string trailing=shellWordsRawText(remainder,words,esacIdx+1,words.size());
		if(savedStdin>=0){
		dup2(savedStdin,0);
		close(savedStdin);
		}
		if(!heredocTempFile.empty())
		unlink(heredocTempFile.c_str());

		// A pending "return"/"break"/"continue" (see Shell::returning()/
		// breaking()/continuing()) must skip any trailing command on
		// the same input too, so it can keep unwinding up to whichever

commands/shell/for.cpp

+32 −0

Changes for commands/shell/for.cpp: 32 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -30,6 +30,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

		#include <iostream>

		#include <fcntl.h>
		#include <unistd.h>

		#include "shell.h"
		#include "envoirment.h"

		@@ -82,6 +85,28 @@ namespace fresh { namespace cmd {
		// still correctly reports 0, matching real shells.
		int status=shell.running() ? 0 : shell.exitCode();

		// "done <<WORD" -- a here-doc redirect on the loop as a whole
		// (its body may well contain a "read" meant to consume it, one
		// line per iteration), not more statements after it (this
		// builtin's other, "; more..." use of `trailing`, below). See
		// while.cpp's matching comment for the full story -- same fix,
		// same reasoning, just here since "for" is exactly as exposed
		// to it as "while" is.
		int savedStdin=-1;
		std::string heredocTempFile;
		HeredocSpec heredocSpec;
		if(extractHeredocSpec(trailing,heredocSpec)){
		heredocTempFile=shell.collectHeredocBody(heredocSpec);
		if(!heredocTempFile.empty()){
		int fd=open(heredocTempFile.c_str(),O_RDONLY);
		if(fd>=0){
		savedStdin=dup(0);
		dup2(fd,0);
		close(fd);
		}
		}
		}

		shell.enterLoop();
		for(size_t i=0; i<items.size() && shell.running() && !shell.returning() &&
		!shell.breaking() && !shell.continuing(); ++i){
		@@ -114,6 +139,13 @@ namespace fresh { namespace cmd {
		}
		shell.exitLoop();

		if(savedStdin>=0){
		dup2(savedStdin,0);
		close(savedStdin);
		}
		if(!heredocTempFile.empty())
		unlink(heredocTempFile.c_str());

		// A pending "return"/"break"/"continue" must skip any trailing
		// command on the same input too, so it can keep unwinding up
		// to whichever construct is waiting to consume it (return: a

commands/shell/if.cpp

+34 −1

Changes for commands/shell/if.cpp: 34 added lines, 1 removed line.
Original line number	Diff line number	Diff line
		@@ -34,6 +34,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

		#include <iostream>

		#include <fcntl.h>
		#include <unistd.h>

		#include "shell.h"

		namespace fresh { namespace cmd {
		@@ -113,10 +116,40 @@ namespace fresh { namespace cmd {
		// silently clobbered. An ordinary untaken if/elif chain with
		// no else still correctly reports 0, matching real shells.
		int status=shell.running() ? 0 : shell.exitCode();

		std::string trailing=shellWordsRawText(remainder,words,fiIdx+1,words.size());

		// "fi <<WORD" -- a here-doc redirect on the whole if/elif/else
		// construct (the taken branch may contain a "read" meant to
		// consume it), not more statements after it (this builtin's
		// other, "; more..." use of `trailing`, below). See
		// while.cpp's matching comment for the full story -- must be
		// applied before `branch` runs, same reasoning as while/for.
		int savedStdin=-1;
		std::string heredocTempFile;
		HeredocSpec heredocSpec;
		if(extractHeredocSpec(trailing,heredocSpec)){
		heredocTempFile=shell.collectHeredocBody(heredocSpec);
		if(!heredocTempFile.empty()){
		int fd=open(heredocTempFile.c_str(),O_RDONLY);
		if(fd>=0){
		savedStdin=dup(0);
		dup2(fd,0);
		close(fd);
		}
		}
		}

		if(!branch.empty())
		status=shell.parse(branch);

		std::string trailing=shellWordsRawText(remainder,words,fiIdx+1,words.size());
		if(savedStdin>=0){
		dup2(savedStdin,0);
		close(savedStdin);
		}
		if(!heredocTempFile.empty())
		unlink(heredocTempFile.c_str());

		// A pending "return"/"break"/"continue" (see Shell::returning()/
		// breaking()/continuing()) must skip any trailing command on
		// the same input too, so it can keep unwinding up to whichever

commands/shell/while.cpp

+45 −0

Changes for commands/shell/while.cpp: 45 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -31,6 +31,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

		#include <iostream>

		#include <fcntl.h>
		#include <unistd.h>

		#include "shell.h"

		namespace fresh { namespace cmd {
		@@ -60,6 +63,35 @@ namespace fresh { namespace cmd {
		std::string trailing=shellWordsRawText(remainder,words,doneIdx+1,words.size());

		Shell &shell=Shell::instance();

		// "done <<WORD" -- a here-doc redirect on the loop AS A WHOLE
		// (e.g. the kernel's own tools/lib/subcmd/../sync-check.sh:
		// "while read -r f; do ...; done <<EOF\n$FILES\nEOF"), not more
		// statements after it (this builtin's other, "; more..." use of
		// `trailing`, handled below). Must be applied to fd 0 BEFORE the
		// loop below ever runs: the loop's own CONDITION is frequently
		// exactly the "read" that's supposed to consume this body (as
		// in the sync-check.sh example above), so redirecting only
		// after the loop returns is already too late -- every previous
		// build of this shell instead left fd 0 pointed at whatever it
		// already was (a real terminal or an unrelated inherited pipe),
		// so that first "read" blocked forever waiting for input that
		// was never coming, or silently consumed the wrong stream.
		int savedStdin=-1;
		std::string heredocTempFile;
		HeredocSpec heredocSpec;
		bool hasHeredoc=extractHeredocSpec(trailing,heredocSpec);
		if(hasHeredoc){
		heredocTempFile=shell.collectHeredocBody(heredocSpec);
		if(!heredocTempFile.empty()){
		int fd=open(heredocTempFile.c_str(),O_RDONLY);
		if(fd>=0){
		savedStdin=dup(0);
		dup2(fd,0);
		close(fd);
		}
		}
		}
		// Default for a loop that genuinely never iterates (condition
		// false first try) -- matching real shells. `condition` is
		// evaluated lazily inside the loop below, not yet here, so
		@@ -107,6 +139,19 @@ namespace fresh { namespace cmd {
		}
		shell.exitLoop();

		// Restore whatever fd 0 really was before this loop redirected
		// it above, and drop the temp file backing the heredoc body --
		// both regardless of how the loop ended (normal exhaustion,
		// break, return, or a fatal error), same as any other
		// redirection's scope never outliving the construct it was
		// attached to.
		if(savedStdin>=0){
		dup2(savedStdin,0);
		close(savedStdin);
		}
		if(!heredocTempFile.empty())
		unlink(heredocTempFile.c_str());

		// Unlike if/for/case (whose condition/subject/list is always
		// evaluated into a separate local BEFORE their own "status"
		// fallback is set up, so a fatal trigger there is already

src/shell.h

+19 −8

Changes for src/shell.h: 19 added lines, 8 removed lines.
Original line number	Diff line number	Diff line
		@@ -506,6 +506,25 @@ namespace fresh {
		// replaces $1.. while the file runs and restores them afterward.
		int useFile(const std::string &path,const std::vector<std::string> &args);

		// Reads lines from _heredocSource up to (not including) a line
		// exactly equal to spec.delimiter, expanding each ($VAR etc.,
		// unless spec.quoted) and writing the result into a fresh temp
		// file. Returns that file's path (caller unlinks it once every
		// stage that might read it has finished), or "" if there is no
		// active source to read from. Public (unlike the rest of this
		// group) because while/for/if/case (see e.g. while.cpp) need it
		// too: runSimple()'s own per-stage heredoc handling only ever sees
		// a "<<WORD" that's still attached to a single simple command's own
		// stage text, but a heredoc can just as legally be attached to one
		// of THESE constructs as a whole ("done <<EOF" etc.) -- runSimple()
		// itself has no idea that's even possible, since by the time it
		// reaches a compound statement's stage text, splitPipelineStages()
		// has already folded the whole condition/body/keyword span into one
		// opaque stage, indistinguishable (to runSimple()) from any other
		// simple command. Each of those builtins calls this directly,
		// exactly the way runSimple() already does for a plain command.
		std::string collectHeredocBody(const HeredocSpec &spec);

		private:
		Shell();
		~Shell();
		@@ -525,14 +544,6 @@ namespace fresh {
		// "" on failure.
		std::string writeHeredocBodyToTempFile(const std::string &body);

		// Reads lines from _heredocSource up to (not including) a line
		// exactly equal to spec.delimiter, expanding each ($VAR etc.,
		// unless spec.quoted) and writing the result into a fresh temp
		// file. Returns that file's path (caller unlinks it once every
		// stage that might read it has finished), or "" if there is no
		// active source to read from.
		std::string collectHeredocBody(const HeredocSpec &spec);

		// Same as collectHeredocBody(), but the body has already been
		// captured (as raw, unprocessed lines) into `rawLines` -- e.g. by
		// splitPipelineStages() finding it still present in the very text