From c0ed19c1261ee281d3c8f36ec3f28fa7328f0f39 Mon Sep 17 00:00:00 2001
From: Lukas Larsson <lukas@erlang.org>
Date: Fri, 2 Sep 2016 14:29:26 +0200
Subject: kernel: Close stdin of commands run in os:cmd

This is needed when running programs that only exit when
stdin has been closed, e.g. 'more'.
---
 lib/kernel/src/os.erl                      | 14 +++++++++++++-
 lib/kernel/test/os_SUITE.erl               | 17 ++++++++++++++---
 lib/kernel/test/os_SUITE_data/Makefile.src |  8 +++++++-
 lib/kernel/test/os_SUITE_data/my_fds.c     |  9 +++++++++
 4 files changed, 43 insertions(+), 5 deletions(-)
 create mode 100644 lib/kernel/test/os_SUITE_data/my_fds.c

(limited to 'lib/kernel')

diff --git a/lib/kernel/src/os.erl b/lib/kernel/src/os.erl
index 05bbf1069e..f8519d3a5e 100644
--- a/lib/kernel/src/os.erl
+++ b/lib/kernel/src/os.erl
@@ -254,7 +254,19 @@ mk_cmd(_,Cmd) ->
     {"/bin/sh -s unix:cmd", [out],
      %% We insert a new line after the command, in case the command
      %% contains a comment character.
-     ["(", unicode:characters_to_binary(Cmd), "\n); echo \"\^D\"\n"],
+     %%
+     %% The </dev/null closes stdin, which means that programs
+     %% that use a closed stdin as an termination indicator works.
+     %% An example of such a program is 'more'.
+     %%
+     %% The "echo ^D" is used to indicate that the program has executed
+     %% and we should return any output we have gotten. We cannot use
+     %% termination of the child or closing of stdin/stdout as then
+     %% starting background jobs from os:cmd will block os:cmd.
+     %%
+     %% I tried changing this to be "better", but got bombarded with
+     %% backwards incompatibility bug reports, so leave this as it is.
+     ["(", unicode:characters_to_binary(Cmd), "\n) </dev/null; echo \"\^D\"\n"],
      <<$\^D>>}.
 
 validate(Atom) when is_atom(Atom) ->
diff --git a/lib/kernel/test/os_SUITE.erl b/lib/kernel/test/os_SUITE.erl
index 19ab3713a1..ef252b732d 100644
--- a/lib/kernel/test/os_SUITE.erl
+++ b/lib/kernel/test/os_SUITE.erl
@@ -25,7 +25,7 @@
 -export([space_in_cwd/1, quoting/1, cmd_unicode/1, space_in_name/1, bad_command/1,
 	 find_executable/1, unix_comment_in_command/1, deep_list_command/1,
          large_output_command/1, background_command/0, background_command/1,
-         message_leak/1, perf_counter_api/1]).
+         message_leak/1, close_stdin/0, close_stdin/1, perf_counter_api/1]).
 
 -include_lib("common_test/include/ct.hrl").
 
@@ -37,7 +37,7 @@ all() ->
     [space_in_cwd, quoting, cmd_unicode, space_in_name, bad_command,
      find_executable, unix_comment_in_command, deep_list_command,
      large_output_command, background_command, message_leak,
-     perf_counter_api].
+     close_stdin, perf_counter_api].
 
 groups() ->
     [].
@@ -54,7 +54,8 @@ init_per_group(_GroupName, Config) ->
 end_per_group(_GroupName, Config) ->
     Config.
 
-init_per_testcase(background_command, Config) ->
+init_per_testcase(TC, Config)
+  when TC =:= background_command; TC =:= close_stdin ->
     case os:type() of
         {win32, _} ->
             {skip,"Should not work on windows"};
@@ -302,6 +303,16 @@ message_leak(_Config) ->
 
     process_flag(trap_exit, false).
 
+%% Test that os:cmd closes stdin of the program that is executed
+close_stdin() ->
+    [{timetrap, {seconds, 5}}].
+close_stdin(Config) ->
+    DataDir = proplists:get_value(data_dir, Config),
+    Fds = filename:join(DataDir, "my_fds"),
+
+    "-1" = os:cmd(Fds).
+
+
 %% Test that the os:perf_counter api works as expected
 perf_counter_api(_Config) ->
 
diff --git a/lib/kernel/test/os_SUITE_data/Makefile.src b/lib/kernel/test/os_SUITE_data/Makefile.src
index 912d0cbcb1..f83f781411 100644
--- a/lib/kernel/test/os_SUITE_data/Makefile.src
+++ b/lib/kernel/test/os_SUITE_data/Makefile.src
@@ -3,7 +3,7 @@ LD = @LD@
 CFLAGS = @CFLAGS@ -I@erl_include@ @DEFS@
 CROSSLDFLAGS = @CROSSLDFLAGS@
 
-PROGS = my_echo@exe@
+PROGS = my_echo@exe@ my_fds@exe@
 
 all: $(PROGS)
 
@@ -12,3 +12,9 @@ my_echo@exe@: my_echo@obj@
 
 my_echo@obj@: my_echo.c
 	$(CC) -c -o my_echo@obj@ $(CFLAGS) my_echo.c
+
+my_fds@exe@: my_fds@obj@
+	$(LD) $(CROSSLDFLAGS) -o my_fds my_fds@obj@ @LIBS@
+
+my_fds@obj@: my_fds.c
+	$(CC) -c -o my_fds@obj@ $(CFLAGS) my_fds.c
diff --git a/lib/kernel/test/os_SUITE_data/my_fds.c b/lib/kernel/test/os_SUITE_data/my_fds.c
new file mode 100644
index 0000000000..704a4d1e1d
--- /dev/null
+++ b/lib/kernel/test/os_SUITE_data/my_fds.c
@@ -0,0 +1,9 @@
+#include <stdio.h>
+
+int
+main(int argc, char** argv)
+{
+    char buff[1];
+    int res = read(stdin, buff, 1);
+    printf("%d", res);
+}
-- 
cgit v1.2.3