svn commit: trunk/busybox: include libbb shell
vda at busybox.net
vda at busybox.net
Wed Feb 20 22:23:25 UTC 2008
Author: vda
Date: 2008-02-20 14:23:24 -0800 (Wed, 20 Feb 2008)
New Revision: 21079
Log:
libbb: introduce and use nonblock_safe_read(). Yay!
Our shells are immune from this nasty O_NONBLOCK now!
function old new delta
nonblock_safe_read - 78 +78
file_get 276 295 +19
generateMTFValues 428 435 +7
read_line_input 1776 1772 -4
preadbuffer 543 450 -93
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 2/2 up/down: 104/-97) Total: 7 bytes
text data bss dec hex filename
615190 715 23924 639829 9c355 busybox_old
615168 715 23924 639807 9c33f busybox_unstripped
Modified:
trunk/busybox/include/libbb.h
trunk/busybox/libbb/lineedit.c
trunk/busybox/libbb/read.c
trunk/busybox/shell/ash.c
trunk/busybox/shell/hush.c
trunk/busybox/shell/msh.c
Changeset:
Modified: trunk/busybox/include/libbb.h
===================================================================
--- trunk/busybox/include/libbb.h 2008-02-20 20:26:42 UTC (rev 21078)
+++ trunk/busybox/include/libbb.h 2008-02-20 22:23:24 UTC (rev 21079)
@@ -475,6 +475,7 @@
extern void *xrealloc(void *old, size_t size);
extern ssize_t safe_read(int fd, void *buf, size_t count);
+extern ssize_t nonblock_safe_read(int fd, void *buf, size_t count);
extern ssize_t full_read(int fd, void *buf, size_t count);
extern void xread(int fd, void *buf, size_t count);
extern unsigned char xread_char(int fd);
@@ -482,6 +483,7 @@
extern char *reads(int fd, char *buf, size_t count);
// Read one line a-la fgets. Reads byte-by-byte.
// Useful when it is important to not read ahead.
+// Bytes are appended to pfx (which must be malloced, or NULL).
extern char *xmalloc_reads(int fd, char *pfx);
extern ssize_t read_close(int fd, void *buf, size_t count);
extern ssize_t open_read_close(const char *filename, void *buf, size_t count);
Modified: trunk/busybox/libbb/lineedit.c
===================================================================
--- trunk/busybox/libbb/lineedit.c 2008-02-20 20:26:42 UTC (rev 21078)
+++ trunk/busybox/libbb/lineedit.c 2008-02-20 22:23:24 UTC (rev 21079)
@@ -1408,9 +1408,9 @@
parse_and_put_prompt(prompt);
while (1) {
- fflush(stdout);
+ fflush(NULL);
- if (safe_read(STDIN_FILENO, &c, 1) < 1) {
+ if (nonblock_safe_read(STDIN_FILENO, &c, 1) < 1) {
/* if we can't read input then exit */
goto prepare_to_die;
}
Modified: trunk/busybox/libbb/read.c
===================================================================
--- trunk/busybox/libbb/read.c 2008-02-20 20:26:42 UTC (rev 21078)
+++ trunk/busybox/libbb/read.c 2008-02-20 22:23:24 UTC (rev 21079)
@@ -20,6 +20,58 @@
return n;
}
+/* Suppose that you are a shell. You start child processes.
+ * They work and eventually exit. You want to get user input.
+ * You read stdin. But what happens if last child switched
+ * its stdin into O_NONBLOCK mode?
+ *
+ * *** SURPRISE! It will affect the parent too! ***
+ * *** BIG SURPRISE! It stays even after child exits! ***
+ *
+ * This is a design bug in UNIX API.
+ * fcntl(0, F_SETFL, fcntl(0, F_GETFL, 0) | O_NONBLOCK);
+ * will set nonblocking mode not only on _your_ stdin, but
+ * also on stdin of your parent, etc.
+ *
+ * In general,
+ * fd2 = dup(fd1);
+ * fcntl(fd2, F_SETFL, fcntl(fd2, F_GETFL, 0) | O_NONBLOCK);
+ * sets both fd1 and fd2 to O_NONBLOCK. This includes cases
+ * where duping is done implicitly by fork() etc.
+ *
+ * We need
+ * fcntl(fd2, F_SETFD, fcntl(fd2, F_GETFD, 0) | O_NONBLOCK);
+ * (note SETFD, not SETFL!) but such thing doesn't exist.
+ *
+ * Alternatively, we need nonblocking_read(fd, ...) which doesn't
+ * require O_NONBLOCK dance at all. Actually, it exists:
+ * n = recv(fd, buf, len, MSG_DONTWAIT);
+ * "MSG_DONTWAIT:
+ * Enables non-blocking operation; if the operation
+ * would block, EAGAIN is returned."
+ * but recv() works only for sockets!
+ *
+ * So far I don't see any good solution, I can only propose
+ * that affected readers should be careful and use this routine,
+ * which detects EAGAIN and uses poll() to wait on the fd.
+ * Thanksfully, poll() doesn't give rat's ass about O_NONBLOCK flag.
+ */
+ssize_t nonblock_safe_read(int fd, void *buf, size_t count)
+{
+ struct pollfd pfd[1];
+ ssize_t n;
+
+ while (1) {
+ n = safe_read(fd, buf, count);
+ if (n >= 0 || errno != EAGAIN)
+ return n;
+ /* fd is in O_NONBLOCK mode. Wait using poll and repeat */
+ pfd[0].fd = fd;
+ pfd[0].events = POLLIN;
+ safe_poll(pfd, 1, -1);
+ }
+}
+
/*
* Read all of the supplied buffer from a file.
* This does multiple reads as necessary.
@@ -93,6 +145,7 @@
// Read one line a-la fgets. Reads byte-by-byte.
// Useful when it is important to not read ahead.
+// Bytes are appended to pfx (which must be malloced, or NULL).
char *xmalloc_reads(int fd, char *buf)
{
char *p;
@@ -106,7 +159,8 @@
p = buf + sz;
sz += 128;
}
- if (safe_read(fd, p, 1) != 1) { /* EOF/error */
+ /* nonblock_safe_read() because we are used by e.g. shells */
+ if (nonblock_safe_read(fd, p, 1) != 1) { /* EOF/error */
if (p == buf) {
/* we read nothing [and buf was NULL initially] */
free(buf);
Modified: trunk/busybox/shell/ash.c
===================================================================
--- trunk/busybox/shell/ash.c 2008-02-20 20:26:42 UTC (rev 21078)
+++ trunk/busybox/shell/ash.c 2008-02-20 22:23:24 UTC (rev 21079)
@@ -5428,7 +5428,7 @@
read:
if (in.fd < 0)
break;
- i = safe_read(in.fd, buf, sizeof(buf));
+ i = nonblock_safe_read(in.fd, buf, sizeof(buf));
TRACE(("expbackq: read returns %d\n", i));
if (i <= 0)
break;
@@ -8678,7 +8678,7 @@
retry:
#if ENABLE_FEATURE_EDITING
if (!iflag || parsefile->fd)
- nr = safe_read(parsefile->fd, buf, BUFSIZ - 1);
+ nr = nonblock_safe_read(parsefile->fd, buf, BUFSIZ - 1);
else {
#if ENABLE_FEATURE_TAB_COMPLETION
line_input_state->path_lookup = pathval();
@@ -8700,9 +8700,11 @@
}
}
#else
- nr = safe_read(parsefile->fd, buf, BUFSIZ - 1);
+ nr = nonblock_safe_read(parsefile->fd, buf, BUFSIZ - 1);
#endif
+#if 0
+/* nonblock_safe_read() handles this problem */
if (nr < 0) {
if (parsefile->fd == 0 && errno == EWOULDBLOCK) {
int flags = fcntl(0, F_GETFL);
@@ -8715,6 +8717,7 @@
}
}
}
+#endif
return nr;
}
@@ -11801,7 +11804,7 @@
backslash = 0;
STARTSTACKSTR(p);
do {
- if (read(0, &c, 1) != 1) {
+ if (nonblock_safe_read(0, &c, 1) != 1) {
status = 1;
break;
}
Modified: trunk/busybox/shell/hush.c
===================================================================
--- trunk/busybox/shell/hush.c 2008-02-20 20:26:42 UTC (rev 21078)
+++ trunk/busybox/shell/hush.c 2008-02-20 22:23:24 UTC (rev 21079)
@@ -1273,10 +1273,10 @@
prompt_str = setup_prompt_string(i->promptmode);
#if ENABLE_FEATURE_EDITING
/* Enable command line editing only while a command line
- * is actually being read; otherwise, we'll end up bequeathing
- * atexit() handlers and other unwanted stuff to our
- * child processes (rob at sysgo.de) */
- r = read_line_input(prompt_str, user_input_buf, BUFSIZ-1, line_input_state);
+ * is actually being read */
+ do {
+ r = read_line_input(prompt_str, user_input_buf, BUFSIZ-1, line_input_state);
+ } while (r == 0); /* repeat if Ctrl-C */
i->eof_flag = (r < 0);
if (i->eof_flag) { /* EOF/error detected */
user_input_buf[0] = EOF; /* yes, it will be truncated, it's ok */
Modified: trunk/busybox/shell/msh.c
===================================================================
--- trunk/busybox/shell/msh.c 2008-02-20 20:26:42 UTC (rev 21078)
+++ trunk/busybox/shell/msh.c 2008-02-20 22:23:24 UTC (rev 21079)
@@ -42,6 +42,7 @@
# define xmalloc(size) malloc(size)
# define msh_main(argc,argv) main(argc,argv)
# define safe_read(fd,buf,count) read(fd,buf,count)
+# define nonblock_safe_read(fd,buf,count) read(fd,buf,count)
# define NOT_LONE_DASH(s) ((s)[0] != '-' || (s)[1])
# define LONE_CHAR(s,c) ((s)[0] == (c) && !(s)[1])
# define ATTRIBUTE_NORETURN __attribute__ ((__noreturn__))
@@ -3376,7 +3377,7 @@
}
for (wp = t->words + 1; *wp; wp++) {
for (cp = global_env.linep; !nl && cp < elinep - 1; cp++) {
- nb = read(0, cp, sizeof(*cp));
+ nb = nonblock_safe_read(0, cp, sizeof(*cp));
if (nb != sizeof(*cp))
break;
nl = (*cp == '\n');
@@ -4799,7 +4800,7 @@
if (i)
lseek(ap->afile, ap->afpos, SEEK_SET);
- i = safe_read(ap->afile, bp->buf, sizeof(bp->buf));
+ i = nonblock_safe_read(ap->afile, bp->buf, sizeof(bp->buf));
if (i <= 0) {
closef(ap->afile);
return 0;
@@ -4830,7 +4831,7 @@
return c;
}
#endif
- i = safe_read(ap->afile, &c, sizeof(c));
+ i = nonblock_safe_read(ap->afile, &c, sizeof(c));
return i == sizeof(c) ? (c & 0x7f) : (closef(ap->afile), 0);
}
@@ -4841,7 +4842,7 @@
{
char c;
- if (read(ap->afile, &c, sizeof(c)) != sizeof(c)) {
+ if (nonblock_safe_read(ap->afile, &c, sizeof(c)) != sizeof(c)) {
close(ap->afile);
c = '\0';
}
More information about the busybox-cvs
mailing list