Enabling support for the FIFO based console in sandbox

Allow up to one bidirectional FIFO/file based console in addition to the default stdin/stdout console that is always registered.

We avoid opening the FIFO files while parsing the options because the whole logic may block if e.g. trying to open the --stdout FIFO and there is no reader in the other end. So instead, we store the --stdout and --stdin file paths given, and we open both sequentially once all the options have been parsed. This also allows us to validate that at most a single pair of --stdin and --stdout paths has been given. e.g.:
term1 $ mkfifo /tmp/bbstdin
term1 $ mkfifo /tmp/bbstdout
term1 $ ./barebox -I /tmp/bbstdin -O /tmp/bbstdout
(blocks until a reader is available in the stdout FIFO)

term2 $ cat /tmp/bbstdout & cat > /tmp/bbstdin
(starts reader and writer, which triggers barebox to continue)

If only one console is activated (CONFIG_CONSOLE_ACTIVATE_ALL=n), the default stdin/stdout console will be preferred instead of the new FIFO/file based, which would need to be activated explicitly later on. e.g.:
barebox@barebox sandbox:/ cs1.active=ioe

Signed-off-by Aleksander Morgado <[email protected]>

---
 arch/sandbox/os/common.c | 48 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 11 deletions(-)

Quoted 24 lines

Allow up to one bidirectional FIFO/file based console in addition to the default stdin/stdout console that is always registered.

We avoid opening the FIFO files while parsing the options because the whole logic may block if e.g. trying to open the --stdout FIFO and there is no reader in the other end. So instead, we store the --stdout and --stdin file paths given, and we open both sequentially once all the options have been parsed. This also allows us to validate that at most a single pair of --stdin and --stdout paths has been given. e.g.:
term1 $ mkfifo /tmp/bbstdin
term1 $ mkfifo /tmp/bbstdout
term1 $ ./barebox -I /tmp/bbstdin -O /tmp/bbstdout
(blocks until a reader is available in the stdout FIFO)

term2 $ cat /tmp/bbstdout & cat > /tmp/bbstdin
(starts reader and writer, which triggers barebox to continue)

If only one console is activated (CONFIG_CONSOLE_ACTIVATE_ALL=n), the default stdin/stdout console will be preferred instead of the new FIFO/file based, which would need to be activated explicitly later on. e.g.:
barebox@barebox sandbox:/ cs1.active=ioe

Quoted 4 lines

Signed-off-by Aleksander Morgado <[email protected]>

---
 arch/sandbox/os/common.c | 48 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 11 deletions(-)

The first patch is ok and I applied it. Before this patch though I'd like to have the attached patch. It merely fixes what we already have: The -I and -O options got broken over time, we ended up using the barebox malloc pool before it was initialized.

So next we have to create a patch for what you want to do: Have an option to create a bidirectional console in barebox. Can we have a --file-console option that takes two path arguments instead of one, with some separator in between?

------------------------8<-----------------------------

From 8e5a534df784ab91ffde55ed5038e66e8dbb36c8 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <[email protected]>
Date: Thu, 1 Jun 2017 10:08:31 +0200
Subject: [PATCH] sandbox: Fix registering file/fifo consoles

barebox_register_console() uses xzalloc which requires the malloc pool to be initialized, so call it during the second option parsing when this is already done.

Signed-off-by Sascha Hauer <[email protected]>

---
 arch/sandbox/os/common.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

arch/sandbox/os/common.c +19 -20

index 192917ac22..8cf0873130 100644
@@ -359,24 +359,6 @@ int main(int argc, char *argv[])
                                exit(1);
                        }
                        break;
-               case 'O':
-                       fd = open(optarg, O_WRONLY);
-                       if (fd < 0) {
-                               perror("open");
-                               exit(1);
-                       }
-
-                       barebox_register_console(-1, fd);
-                       break;
-               case 'I':
-                       fd = open(optarg, O_RDWR);
-                       if (fd < 0) {
-                               perror("open");
-                               exit(1);
-                       }
-
-                       barebox_register_console(fd, -1);
-                       break;
                case 'x':
                        sdl_xres = strtoul(optarg, NULL, 0);
                        break;

@@ -384,7 +366,7 @@ int main(int argc, char *argv[])
                        sdl_yres = strtoul(optarg, NULL, 0);
                        break;
                default:
-                       exit(1);
+                       break;
                }
        }
 

@@ -421,6 +403,24 @@ int main(int argc, char *argv[])
                        if (ret)
                                exit(1);
                        break;
+               case 'O':
+                       fd = open(optarg, O_WRONLY);
+                       if (fd < 0) {
+                               perror("open");
+                               exit(1);
+                       }
+
+                       barebox_register_console(-1, fd);
+                       break;
+               case 'I':
+                       fd = open(optarg, O_RDWR);
+                       if (fd < 0) {
+                               perror("open");
+                               exit(1);
+                       }
+
+                       barebox_register_console(fd, -1);
+                       break;
                default:
                        break;
                }
--

2.11.0

--

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Quoted 30 lines · Aleksander Morgado

Quoted 24 lines

Allow up to one bidirectional FIFO/file based console in addition to the default stdin/stdout console that is always registered.

We avoid opening the FIFO files while parsing the options because the whole logic may block if e.g. trying to open the --stdout FIFO and there is no reader in the other end. So instead, we store the --stdout and --stdin file paths given, and we open both sequentially once all the options have been parsed. This also allows us to validate that at most a single pair of --stdin and --stdout paths has been given. e.g.:
term1 $ mkfifo /tmp/bbstdin
term1 $ mkfifo /tmp/bbstdout
term1 $ ./barebox -I /tmp/bbstdin -O /tmp/bbstdout
(blocks until a reader is available in the stdout FIFO)

term2 $ cat /tmp/bbstdout & cat > /tmp/bbstdin
(starts reader and writer, which triggers barebox to continue)

If only one console is activated (CONFIG_CONSOLE_ACTIVATE_ALL=n), the default stdin/stdout console will be preferred instead of the new FIFO/file based, which would need to be activated explicitly later on. e.g.:
barebox@barebox sandbox:/ cs1.active=ioe

Quoted 4 lines

Signed-off-by Aleksander Morgado <[email protected]>

---
 arch/sandbox/os/common.c | 48 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 11 deletions(-)

Quoted 5 lines

The first patch is ok and I applied it. Before this patch though I'd like to have the attached patch. It merely fixes what we already have: The -I and -O options got broken over time, we ended up using the barebox malloc pool before it was initialized.

Ah! I see, just moving the 'O' and 'I' parsing to the second getopt makes it work. I actually knew about the malloc pool being prepared in between both getopt runs, as my attempts to strdup() the file paths in the first run didn't succeed, I just didn't realize it was also affecting the other logic :)

Quoted 5 lines

So next we have to create a patch for what you want to do: Have an option to create a bidirectional console in barebox. Can we have a --file-console option that takes two path arguments instead of one, with some separator in between?

Well, is this even needed any more? I'm fine having separate in/out consoles registered, as long as they work.