From mboxrd@z Thu Jan 1 00:00:00 1970
X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org
X-Spam-Level:
X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,UNPARSEABLE_RELAY
autolearn=ham autolearn_force=no version=3.4.4
Received: (qmail 17934 invoked from network); 18 Aug 2021 19:28:25 -0000
Received: from zero.zsh.org (2a02:898:31:0:48:4558:7a:7368)
by inbox.vuxu.org with ESMTPUTF8; 18 Aug 2021 19:28:25 -0000
ARC-Seal: i=1; cv=none; a=rsa-sha256; d=zsh.org; s=rsa-20210803; t=1629314905;
b=C/Qqj/bm3yzcl/TsnOCZqZOHcZde8LpuNBJB8ets/SWZvmtdAgDSwvRpBPA9LUJjtk/smzgyNj
MUSJKw+5jYNNIjm7wOLCyN5YfBdEjlYSloYDqTFqZ4evXPnIGEXMd+7luNE5G/TdaX+VXrI8Xn
U5CSIr8yEiH/6Mthv/RQa8qKMd5lqec9av37XmrQTd6qsTiiez0BCtAsYQMaxSCTHW5FHNyX1t
WizGeCuy/Xj0YqveLOqm035izQn08qL60Vr0Ty1qtYRU/HtSV3KE+abgn5DG8E1LQdGg8/MXCu
zgYrPw9a/jfzHtW+8i9kTLG1Fj85zEaS65dSjbaBqGMp4A==;
ARC-Authentication-Results: i=1; zsh.org;
iprev=pass (smtpq2.tb.ukmail.iss.as9143.net) smtp.remote-ip=212.54.57.97;
dkim=pass header.d=ntlworld.com header.s=meg.feb2017 header.a=rsa-sha256;
dmarc=pass header.from=ntlworld.com;
arc=none
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed; d=zsh.org; s=rsa-20210803; t=1629314905;
bh=BMT0Gb0Wvf/eeBha0tbuljwqFirlfUA/uGfWs7k4ncU=;
h=List-Archive:List-Owner:List-Post:List-Unsubscribe:List-Subscribe:List-Help:
List-Id:Sender:Content-Transfer-Encoding:MIME-Version:Content-Type:
References:In-Reply-To:Date:To:From:Subject:Message-ID:DKIM-Signature:
DKIM-Signature;
b=ivsQ4OlCeDAltRE73gtqZXBOTO3wgJSnJOVwcwvruiaMIDYyCoAro6SKTlA8ACE0nVjRZjAJzM
jTGqSeLdjlXhJaUscgWOLYcVmgfBsaclGYRdItKwb19Mg2B5TTQXImlrGqsT+D0QQRhrCw3Mqz
+ksaaqMH/bi0QuO74GqnyRZvHQaLDARPTWlBfLyIJgD0Jrem+TRfjzhVd3nfPHmlt5LXr+m27G
VWGmCMlLR9I3UM7A8e0E64DpTkhg6AJkTcJRmv8EvSQ+CEKAx70yu/AlMWDBs0+m5M4FO4OrDz
S3UNydINLqSbVJwnqWDZgyORNB21ObAR7k+OYHHwz6mtZQ==;
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=zsh.org;
s=rsa-20210803; h=List-Archive:List-Owner:List-Post:List-Unsubscribe:
List-Subscribe:List-Help:List-Id:Sender:Content-Transfer-Encoding:
Mime-Version:Content-Type:References:In-Reply-To:Date:To:From:Subject:
Message-ID:Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From
:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID;
bh=X6uDWFH0Ltse59sbEvVCz0fZDG7dUvm8ALV98Ywwfts=; b=g4QezInTZYk2zOuX/jyPTcJh4E
WfKGz6ANxjeLdTsEMoc2NdYnTCuLRECY61kabPJv78lTaAVNSm1ks/r55XZymSCVt9L6FIMFEYu2R
z6tBfVmOzOP5a+LsDHCs7oAAD3KqbAjt87++uaJftgCvi8DD4XO6yEyKolSR4T8h1H9HzJu9Gka0Q
jty1kUDWAwwdKHJw+4P/dy7ji+QRa76IX+815a84TDcodVpvB1iA5Ig9SOcPNBlSlKIytpDJ9Vxi8
t/j22QQ/z5GnQuLaUEgg8MN/wp7auUKGNz3KBLw/kA1jE1F/SkuXNEDq7taM1JMQq8NqRAHsbnOHi
JFzmhlUA==;
Received: from authenticated user by zero.zsh.org with local
id 1mGREd-000Mgy-PI; Wed, 18 Aug 2021 19:28:23 +0000
Authentication-Results: zsh.org;
iprev=pass (smtpq2.tb.ukmail.iss.as9143.net) smtp.remote-ip=212.54.57.97;
dkim=pass header.d=ntlworld.com header.s=meg.feb2017 header.a=rsa-sha256;
dmarc=pass header.from=ntlworld.com;
arc=none
Received: from smtpq2.tb.ukmail.iss.as9143.net ([212.54.57.97]:60810)
by zero.zsh.org with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128)
id 1mGRE4-000MN5-EJ; Wed, 18 Aug 2021 19:27:49 +0000
Received: from [212.54.57.107] (helo=csmtp3.tb.ukmail.iss.as9143.net)
by smtpq2.tb.ukmail.iss.as9143.net with esmtp (Exim 4.86_2)
(envelope-from
)
id 1mGRE4-0004x9-2X
for zsh-workers@zsh.org; Wed, 18 Aug 2021 21:27:48 +0200
Received: from pws-Zeus ([86.7.189.51])
by cmsmtp with ESMTPA
id GRE3mtaTLbkrTGRE3m4xr0; Wed, 18 Aug 2021 21:27:47 +0200
X-Originating-IP: [86.7.189.51]
X-Authenticated-Sender: p.w.stephenson@ntlworld.com
X-Spam: 0
X-Authority: v=2.4 cv=OYD7sjfY c=1 sm=1 tr=0 ts=611d5f33 cx=a_exe
a=mDzK/eG20+r+ucIvzJc7BQ==:117 a=mDzK/eG20+r+ucIvzJc7BQ==:17
a=IkcTkHD0fZMA:10 a=MhDmnRu9jo8A:10 a=6ZuCY_HIQIYlpt_9tuwA:9 a=QEXdDO2ut3YA:10
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ntlworld.com;
s=meg.feb2017; t=1629314867;
bh=X6uDWFH0Ltse59sbEvVCz0fZDG7dUvm8ALV98Ywwfts=;
h=Subject:From:To:Date:In-Reply-To:References;
b=llIh6wDpHFY3S1uh3JFqmKQjkhpE9+4xPUnuaxu+S9FHYlj494qowEeSSKTJNJs61
1RdFYHaKFnFuV+8AjJ65lCK7pOveLzUS1F5sz/SNYWZw2DTQhm7vNvFp1hnhdvXKvt
yJtzm11rfr+BR5viOC/TJjPtpA0g9X2kf2aPyw2fKqNXIFq0K0VDeIMGWGW12pJvBC
gv3kp55Pt2pfc3fZWMwVz2p0bor+XloUwfy0xQcZC4OxUVsZpSnDIEQtNm6Xvxha1Z
Df4vhXHHKxhB1khapxprJ/vpxXoV/76LRJuH4CF6Wi6wYaACSG7KQFfnmuL+CifEAw
BmMplZmoDvxHw==
Message-ID: <5509351db61721486dc9d27c7d816e8bea23b829.camel@ntlworld.com>
Subject: Re: [BUG] malloc inside signal handler
From: Peter Stephenson
To: zsh-workers@zsh.org
Date: Wed, 18 Aug 2021 20:27:47 +0100
In-Reply-To: <1888954006.948395.1629188495083@mail2.virginmedia.com>
References: <20210724225048.ly7qpdvi55j3h5mi@localhost>
<20210731202547.yxzvwevxvy22ywm3@localhost>
<1888954006.948395.1629188495083@mail2.virginmedia.com>
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
X-CMAE-Envelope: MS4xfFrZhfGR5lGor77yIO/HwsnICPZcP3sI3wYvO4W8tJNUI4NasrdTUOZM/J21st2a6FQOOi6lShNW7DCguuz4WVHqSqx5WzuVzmVSEKbGe9V/jvGqmrLE
Oqmd6M9U75OpIXdXxA7wsRW8oLXGtZCwQW3YQVFM7H2Cb41T3FqOOiYUjHa4PjVCLZqJ1/s+u7v1Pw==
X-Seq: 49290
Archived-At:
X-Loop: zsh-workers@zsh.org
Errors-To: zsh-workers-owner@zsh.org
Precedence: list
Precedence: bulk
Sender: zsh-workers-request@zsh.org
X-no-archive: yes
List-Id:
List-Help:
List-Subscribe:
List-Unsubscribe:
List-Post:
List-Owner:
List-Archive:
On Tue, 2021-08-17 at 09:21 +0100, Peter Stephenson wrote:
> Avoiding stdio for input of this sort doesn't actually look infeasible,
> either --- mentions of bshin (buffered shell input from files) are
> restricted to only a couple of files. But it's kind of annoying as this
> is exactly the job stdio is designed for.
As I noted, this isn't that difficult, and this should remove the
problem --- all the memory management is now in places where we would
expect it.
I don't know what the general feeling is about this, but as a piece of
long-term robustness and with no immediately upcoming release I'd
incline to getting this or something like it in for trying out.
It could usefully do with a scan for leaks, which is the most likely
thing I can see going wrong. Obviously I've checked myself, but
potentially there are possible leaks of file descriptors, stack entries,
and allocated buffers, so it's not completely trivial.
In case it's not clear: the allocation of the buffer is not tied to the
file descriptor in use. The only time we need to allocate a new buffer
other than at initialisation is when we're temporarily saving the
existing input while we source a different file. Otherwise, we simply
need to reset the buffer pointers.
pws
diff --git a/Src/init.c b/Src/init.c
index 3d6c94d04..878a53a37 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1229,7 +1229,7 @@ setupshin(char *runscript)
/*
* Finish setting up SHIN and its relatives.
*/
- bshin = SHIN ? fdopen(SHIN, "r") : stdin;
+ shinbufalloc();
if (isset(SHINSTDIN) && !SHIN && unset(INTERACTIVE)) {
#ifdef _IONBF
setvbuf(stdin, NULL, _IONBF, 0);
@@ -1384,9 +1384,9 @@ init_misc(char *cmd, char *zsh_name)
dosetopt(RESTRICTED, 1, 0, opts);
if (cmd) {
if (SHIN >= 10)
- fclose(bshin);
+ close(SHIN);
SHIN = movefd(open("/dev/null", O_RDONLY | O_NOCTTY));
- bshin = fdopen(SHIN, "r");
+ shinbufreset();
execstring(cmd, 0, 1, "cmdarg");
stopmsg = 1;
zexit((exit_pending || shell_exiting) ? exit_val : lastval, ZEXIT_NORMAL);
@@ -1409,7 +1409,6 @@ source(char *s)
int tempfd = -1, fd, cj;
zlong oldlineno;
int oldshst, osubsh, oloops;
- FILE *obshin;
char *old_scriptname = scriptname, *us;
char *old_scriptfilename = scriptfilename;
unsigned char *ocs;
@@ -1426,7 +1425,6 @@ source(char *s)
/* save the current shell state */
fd = SHIN; /* store the shell input fd */
- obshin = bshin; /* store file handle for buffered shell input */
osubsh = subsh; /* store whether we are in a subshell */
cj = thisjob; /* store our current job number */
oldlineno = lineno; /* store our current lineno */
@@ -1439,7 +1437,7 @@ source(char *s)
if (!prog) {
SHIN = tempfd;
- bshin = fdopen(SHIN, "r");
+ shinbufsave();
}
subsh = 0;
lineno = 1;
@@ -1507,10 +1505,10 @@ source(char *s)
if (prog)
freeeprog(prog);
else {
- fclose(bshin);
+ close(SHIN);
fdtable[SHIN] = FDT_UNUSED;
SHIN = fd; /* the shell input fd */
- bshin = obshin; /* file handle for buffered shell input */
+ shinbufrestore();
}
subsh = osubsh; /* whether we are in a subshell */
thisjob = cj; /* current job number */
diff --git a/Src/input.c b/Src/input.c
index f568cc135..3c6ad7002 100644
--- a/Src/input.c
+++ b/Src/input.c
@@ -80,11 +80,6 @@
/**/
int SHIN;
-/* buffered shell input for non-interactive shells */
-
-/**/
-FILE *bshin;
-
/* != 0 means we are reading input from a string */
/**/
@@ -129,7 +124,116 @@ static struct instacks *instack, *instacktop;
static int instacksz = INSTACK_INITIAL;
-/* Read a line from bshin. Convert tokens and *
+/* Size of buffer for non-interactive command input */
+
+#define SHINBUFSIZE 8192
+
+/* Input buffer for non-interactive command input */
+static char *shinbuffer;
+
+/* Pointer into shinbuffer */
+static char *shinbufptr;
+
+/* End of contents read into shinbuffer */
+static char *shinbufendptr;
+
+/* Entry on SHIN buffer save stack */
+struct shinsaveentry {
+ /* Next entry on stack */
+ struct shinsaveentry *next;
+ /* Saved shinbuffer */
+ char *buffer;
+ /* Saved shinbufptr */
+ char *ptr;
+ /* Saved shinbufendptr */
+ char *endptr;
+};
+
+/* SHIN buffer save stack */
+struct shinsaveentry *shinsavestack;
+
+/* Reset the input buffer for SHIN, discarding any pending input */
+
+/**/
+void
+shinbufreset(void)
+{
+ shinbufendptr = shinbufptr = shinbuffer;
+}
+
+/* Allocate a new shinbuffer
+ *
+ * Only called at shell initialisation and when saving on the stack.
+ */
+
+/**/
+void
+shinbufalloc(void)
+{
+ shinbuffer = zalloc(SHINBUFSIZE);
+ shinbufreset();
+}
+
+/* Save entry on SHIN buffer save stack */
+
+/**/
+void
+shinbufsave(void)
+{
+ struct shinsaveentry *entry =
+ (struct shinsaveentry *)zalloc(sizeof(struct shinsaveentry));
+
+ entry->next = shinsavestack;
+ entry->buffer = shinbuffer;
+ entry->ptr = shinbufptr;
+ entry->endptr = shinbufendptr;
+
+ shinsavestack = entry;
+
+ shinbufalloc();
+}
+
+/* Restore entry from SHIN buffer save stack */
+
+/**/
+void
+shinbufrestore(void)
+{
+ struct shinsaveentry *entry = shinsavestack;
+
+ zfree(shinbuffer, SHINBUFSIZE);
+
+ shinbuffer = entry->buffer;
+ shinbufptr = entry->ptr;
+ shinbufendptr = entry->endptr;
+
+ shinsavestack = entry->next;
+ zfree(entry, sizeof(struct shinsaveentry));
+}
+
+/* Get a character from SHIN, -1 if none available */
+
+/**/
+static int
+shingetchar(void)
+{
+ int nread;
+
+ if (shinbufptr < shinbufendptr)
+ return STOUC(*shinbufptr++);
+
+ shinbufreset();
+ do {
+ errno = 0;
+ nread = read(SHIN, shinbuffer, SHINBUFSIZE);
+ } while (nread < 0 && errno == EINTR);
+ if (nread <= 0)
+ return -1;
+ shinbufendptr = shinbuffer + nread;
+ return STOUC(*shinbufptr++);
+}
+
+/* Read a line from SHIN. Convert tokens and *
* null characters to Meta c^32 character pairs. */
/**/
@@ -147,11 +251,7 @@ shingetline(void)
winch_unblock();
dont_queue_signals();
for (;;) {
- /* Can't fgets() here because we need to accept '\0' bytes */
- do {
- errno = 0;
- c = fgetc(bshin);
- } while (c < 0 && errno == EINTR);
+ c = shingetchar();
if (c < 0 || c == '\n') {
winch_block();
restore_queue_signals(q);