On Thursday 27 March 2008 13:08, Denys Vlasenko wrote:
> I assume you do this:
>
> mkfifo cmd_pipe
> setsid fbsplash -f cmd_pipe .... &
> ...
> echo 33 >cmd_pipe
> ...
> echo 66 >cmd_pipe
> ...
>
> It will not work. Pipe's input side must not be closed!
>
> mkfifo cmd_pipe
> setsid fbsplash -f cmd_pipe .... &
> {
> ...
> echo 33
> ...
> echo 66
> ...
> } >cmd_pipe
Hmm. Your patch did this:
+ while(1) {
+ int nVal;
+
+ len = read(fd, buf, 255);
+ if (len == -1) {
+ DEBUG_MESSAGE("error reading the fifo");
+ break;
+ }
...
+ }
So it did not abort on EOF - if pipe was closed on the input side,
it would loop, returning to read() and reading 0 bytes again and again
(and again and again and again and again...) until somebody will
open input side again and write someting (e.g. "exit") there,
or task will be killed.
From one POV, it's useful behaviour (you do not need to preserve open fd
on input side), OTOH eating CPU like crazy is not polite too :)
The solution might be to close/open output side (open will block
until there is a writer).
Please try this patch.
--
vda
diff -d -urpN busybox.5/include/usage.h busybox.6/include/usage.h
--- busybox.5/include/usage.h 2008-03-26 21:02:49.000000000 +0100
+++ busybox.6/include/usage.h 2008-03-27 14:03:11.000000000 +0100
@@ -133,8 +133,6 @@
"\n -f Control pipe (else exit after drawing image)" \
"\n commands: 'NN' (% for progress bar) or 'exit'" \
-
-
#define brctl_trivial_usage \
"COMMAND [BRIDGE [INTERFACE]]"
#define brctl_full_usage \
diff -d -urpN busybox.5/miscutils/Config.in busybox.6/miscutils/Config.in
--- busybox.5/miscutils/Config.in 2008-03-26 16:09:58.000000000 +0100
+++ busybox.6/miscutils/Config.in 2008-03-27 14:04:28.000000000 +0100
@@ -214,13 +214,13 @@ config FBSPLASH
-c: hide cursor
-d /dev/fbN: framebuffer device (if not /dev/fb0)
-s path_to_image_file (can be "-" for stdin)
- -i path_to_cfg_file
+ -i path_to_cfg_file (can be "-" for stdin)
-f path_to_fifo (can be "-" for stdin)
- if you want to run it only in presence of kernel parameter:
grep -q "fbsplash=on" </proc/cmdline && setsid fbsplash [params] &
- commands for fifo:
"NN" (ASCII decimal number) - percentage to show on progress bar
- "exit" (or just close fifo) - well you guessed it
+ "exit" - well you guessed it
config LAST
bool "last"
diff -d -urpN busybox.5/miscutils/fbsplash.c busybox.6/miscutils/fbsplash.c
--- busybox.5/miscutils/fbsplash.c 2008-03-26 21:03:30.000000000 +0100
+++ busybox.6/miscutils/fbsplash.c 2008-03-27 14:05:06.000000000 +0100
@@ -394,7 +394,8 @@ int fbsplash_main(int argc ATTRIBUTE_UNU
fb_drawimage();
- if (fifo_filename) {
+ if (fifo_filename) while (1) {
+ struct stat statbuf;
unsigned num;
char *num_buf;
@@ -402,11 +403,16 @@ int fbsplash_main(int argc ATTRIBUTE_UNU
// Block on read, waiting for some input.
// Use of <stdio.h> style I/O allows to correctly
// handle a case when we have many buffered lines
- // already in the pipe.
+ // already in the pipe
while ((num_buf = xmalloc_fgetline(fp)) != NULL) {
if (strncmp(num_buf, "exit", 4) == 0) {
DEBUG_MESSAGE("exit");
- break;
+ exit_cmd:
+ if (bCursorOff) {
+ // restore cursor
+ full_write(STDOUT_FILENO, "\x1b" "[?25h", 6);
+ }
+ return EXIT_SUCCESS;
}
num = atoi(num_buf);
if (isdigit(num_buf[0]) && (num <= 100)) {
@@ -419,13 +425,28 @@ int fbsplash_main(int argc ATTRIBUTE_UNU
}
free(num_buf);
}
- if (bCursorOff) {
- // restore cursor
- full_write(STDOUT_FILENO, "\x1b" "[?25h", 6);
+ // We got EOF/error on fp
+ if (ferror(fp))
+ goto exit_cmd;
+ fclose(fp);
+ if (LONE_DASH(fifo_filename)
+ || stat(fifo_filename, &statbuf) != 0
+ || !S_ISFIFO(statbuf.st_mode)
+ ) {
+ goto exit_cmd;
}
- if (ENABLE_FEATURE_CLEAN_UP)
- fclose(fp);
- }
+ // It's really a named pipe!
+ // For named pipes, we want to support this:
+ // mkfifo cmd_pipe
+ // fbsplash -f cmd_pipe .... &
+ // ...
+ // echo 33 >cmd_pipe
+ // ...
+ // echo 66 >cmd_pipe
+ // This means that on EOF, we need to close/open cmd_pipe
+ // (just reading again works too, but it hogs CPU)
+ fp = xfopen_stdin(fifo_filename); // blocks on open
+ } // end of while (1)
return EXIT_SUCCESS;
}
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
