From 5c2c0b8b36f6c5c53b7e922f115248b884648f30 Mon Sep 17 00:00:00 2001 From: =?utf8?q?R=C3=A9mi=20Denis-Courmont?= Date: Sat, 24 Mar 2007 11:08:46 +0000 Subject: [PATCH] - Fix various error handling bugs in vlc_execve. - Use a single pipe rather two pairs --- src/extras/libc.c | 197 +++++++++++++++++++++++----------------------- 1 file changed, 99 insertions(+), 98 deletions(-) diff --git a/src/extras/libc.c b/src/extras/libc.c index 703d92071b..2e347d1aa6 100644 --- a/src/extras/libc.c +++ b/src/extras/libc.c @@ -25,11 +25,12 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA. *****************************************************************************/ +#include + #include /* strdup() */ #include #include -#include #undef iconv_t #undef iconv_open @@ -49,6 +50,9 @@ # include # include # include +# include +# include +# include #endif #if defined(WIN32) || defined(UNDER_CE) @@ -914,128 +918,122 @@ char **vlc_parse_cmdline( const char *psz_cmdline, int *i_args ) * vlc_execve: Execute an external program with a given environment, * wait until it finishes and return its standard output *************************************************************************/ -int __vlc_execve( vlc_object_t *p_object, int i_argc, char **ppsz_argv, - char **ppsz_env, char *psz_cwd, char *p_in, int i_in, - char **pp_data, int *pi_data ) +int __vlc_execve( vlc_object_t *p_object, int i_argc, char *const *ppsz_argv, + char *const *ppsz_env, const char *psz_cwd, + const char *p_in, size_t i_in, + char **pp_data, size_t *pi_data ) { #ifdef HAVE_FORK - int pi_stdin[2]; - int pi_stdout[2]; - pid_t i_child_pid; +# define BUFSIZE 1024 + int fds[2], i_status; - pipe( pi_stdin ); - pipe( pi_stdout ); - - if ( (i_child_pid = fork()) == -1 ) - { - msg_Err( p_object, "unable to fork (%s)", strerror(errno) ); + if (socketpair (AF_LOCAL, SOCK_STREAM, 0, fds)) return -1; - } - if ( i_child_pid == 0 ) - { - close(0); - dup(pi_stdin[1]); - close(pi_stdin[0]); + pid_t pid = -1; + if ((fds[0] > 2) && (fds[1] > 2)) + pid = fork (); - close(1); - dup(pi_stdout[1]); - close(pi_stdout[0]); + switch (pid) + { + case -1: + msg_Err (p_object, "unable to fork (%s)", strerror (errno)); + close (fds[0]); + close (fds[1]); + return -1; - close(2); + case 0: + /* NOTE: + * Like it or not, close can fail (and not only with EBADF) + */ + if ((close (0) == 0) && (close (1) == 0) && (close (2) == 0) + && (dup (fds[1]) == 0) && (dup (fds[1]) == 1) + && (open ("/dev/null", O_RDONLY) == 2) + && ((psz_cwd == NULL) || (chdir (psz_cwd) == 0))) + execve (ppsz_argv[0], ppsz_argv, ppsz_env); - if ( psz_cwd != NULL ) - chdir( psz_cwd ); - execve( ppsz_argv[0], ppsz_argv, ppsz_env ); - exit(1); + exit (1); } - close(pi_stdin[1]); - close(pi_stdout[1]); - if ( !i_in ) - close( pi_stdin[0] ); + close (fds[1]); *pi_data = 0; - if( *pp_data ) - free( *pp_data ); + if (*pp_data) + free (*pp_data); *pp_data = NULL; - *pp_data = malloc( 1025 ); /* +1 for \0 */ - if( !*pp_data ) - return -1; - while ( !p_object->b_die ) + if (i_in == 0) + shutdown (fds[0], SHUT_WR); + + while (!p_object->b_die) { - int i_ret, i_status; - fd_set readfds, writefds; - struct timeval tv; - - FD_ZERO( &readfds ); - FD_ZERO( &writefds ); - FD_SET( pi_stdout[0], &readfds ); - if ( i_in ) - FD_SET( pi_stdin[0], &writefds ); - - tv.tv_sec = 0; - tv.tv_usec = 10000; - - i_ret = select( pi_stdin[0] > pi_stdout[0] ? pi_stdin[0] + 1 : - pi_stdout[0] + 1, &readfds, &writefds, NULL, &tv ); - if ( i_ret > 0 ) + struct pollfd ufd[1]; + memset (ufd, 0, sizeof (ufd)); + ufd[0].fd = fds[0]; + ufd[0].events = POLLIN; + + if (i_in > 0) + ufd[0].events |= POLLOUT; + + if (poll (ufd, 1, 10) <= 0) + continue; + + if (ufd[0].revents & ~POLLOUT) { - if ( FD_ISSET( pi_stdout[0], &readfds ) ) - { - ssize_t i_read = read( pi_stdout[0], &(*pp_data)[*pi_data], - 1024 ); - if ( i_read > 0 ) - { - *pi_data += i_read; - *pp_data = realloc( *pp_data, *pi_data + 1025 ); - } - } - if ( FD_ISSET( pi_stdin[0], &writefds ) ) + char *ptr = realloc (*pp_data, *pi_data + BUFSIZE + 1); + if (ptr == NULL) + break; /* safely abort */ + + *pp_data = ptr; + + ssize_t val = read (fds[0], ptr + *pi_data, BUFSIZE); + switch (val) { - ssize_t i_write = write( pi_stdin[0], p_in, __MIN(i_in, 1024) ); - - if ( i_write > 0 ) - { - p_in += i_write; - i_in -= i_write; - } - if ( !i_in ) - close( pi_stdin[0] ); + case -1: + case 0: + shutdown (fds[0], SHUT_RD); + break; + + default: + *pi_data += val; } } - if ( waitpid( i_child_pid, &i_status, WNOHANG ) == i_child_pid ) + if (ufd[0].revents & POLLOUT) { - if ( WIFEXITED( i_status ) ) + ssize_t val = write (fds[0], p_in, i_in); + switch (val) { - if ( WEXITSTATUS( i_status ) ) - { - msg_Warn( p_object, - "child %s returned with error code %d", - ppsz_argv[0], WEXITSTATUS( i_status ) ); - } + case -1: + case 0: + i_in = 0; + shutdown (fds[0], SHUT_WR); + break; + + default: + i_in -= val; + p_in += val; } - else - { - if ( WIFSIGNALED( i_status ) ) - { - msg_Warn( p_object, - "child %s quit on signal %d", ppsz_argv[0], - WTERMSIG( i_status ) ); - } - } - if ( i_in ) - close( pi_stdin[0] ); - close( pi_stdout[0] ); - break; } + } - if ( i_ret < 0 && errno != EINTR ) - { - msg_Warn( p_object, "select failed (%s)", strerror(errno) ); - } + close (fds[0]); + + while (waitpid (pid, &i_status, 0) == -1); + + if (WIFEXITED (i_status)) + { + i_status = WEXITSTATUS (i_status); + if (i_status) + msg_Warn (p_object, "child %s (PID %d) exited with error code %d", + ppsz_argv[0], (int)pid, i_status); + } + else + if (WIFSIGNALED (i_status)) // <-- this should be redumdant a check + { + i_status = WTERMSIG (i_status); + msg_Warn (p_object, "child %s (PID %d) exited on signal %d (%s)", + ppsz_argv[0], (int)pid, i_status, strsignal (i_status)); } #elif defined( WIN32 ) && !defined( UNDER_CE ) @@ -1210,6 +1208,9 @@ int __vlc_execve( vlc_object_t *p_object, int i_argc, char **ppsz_argv, #endif + if (*pp_data == NULL) + return -1; + (*pp_data)[*pi_data] = '\0'; return 0; } -- 2.39.2