diff --git a/common.cpp b/common.cpp index 049cdf342..d76b7a46c 100644 --- a/common.cpp +++ b/common.cpp @@ -635,7 +635,7 @@ int read_blocked(int fd, void *buf, size_t count) ssize_t write_loop(int fd, const char *buff, size_t count) { - size_t out=0; + ssize_t out=0; size_t out_cum=0; while( 1 ) { @@ -644,12 +644,12 @@ ssize_t write_loop(int fd, const char *buff, size_t count) count - out_cum ); if (out < 0) { - if( errno != EAGAIN && - errno != EINTR ) + if(errno != EAGAIN && errno != EINTR) { return -1; } - } else + } + else { out_cum += (size_t)out; } @@ -686,6 +686,79 @@ void debug( int level, const wchar_t *msg, ... ) errno = errno_old; } +void debug_safe(int level, const char *msg, const char *param1, const char *param2, const char *param3) +{ + if (! msg) + return; + + /* Can't call printf, that may allocate memory Just call write() over and over. */ + if (level > debug_level) + return; + int errno_old = errno; + + size_t param_idx = 0; + const char *cursor = msg; + while (*cursor != '\0') { + const char *end = strchr(cursor, '%'); + if (end == NULL) + end = cursor + strlen(cursor); + + write(STDERR_FILENO, cursor, end - cursor); + + if (end[0] == '%' && end[1] == 's') { + /* Handle a format string */ + const char *format = NULL; + switch (param_idx++) { + case 0: format = param1; break; + case 1: format = param2; break; + case 2: format = param3; break; + } + if (! format) + format = "(null)"; + write(STDERR_FILENO, format, strlen(format)); + cursor = end + 2; + } else if (end[0] == '\0') { + /* Must be at the end of the string */ + cursor = end; + } else { + /* Some other format specifier, just skip it */ + cursor = end + 1; + } + } + + // We always append a newline + write(STDERR_FILENO, "\n", 1); + + errno = errno_old; +} + +void format_int_safe(char buff[128], int val) { + if (val == 0) { + strcpy(buff, "0"); + } else { + /* Generate the string in reverse */ + size_t idx = 0; + bool negative = (val < 0); + if (negative) + val = -val; + + while (val > 0) { + buff[idx++] = val % 10; + val /= 10; + } + if (negative) + buff[idx++] = '-'; + buff[idx++] = 0; + + size_t left = 0, right = idx - 1; + while (left < right) { + char tmp = buff[left]; + buff[left++] = buff[right]; + buff[right--] = tmp; + } + } +} + void write_screen( const wcstring &msg, wcstring &buff ) { const wchar_t *start, *pos; @@ -1979,10 +2052,9 @@ void sb_format_size( string_buffer_t *sb, wcstring format_size(long long sz) { wcstring result; - const wchar_t *sz_name[]= - { + const wchar_t *sz_name[]= { L"kB", L"MB", L"GB", L"TB", L"PB", L"EB", L"ZB", L"YB", 0 - }; + }; if( sz < 0 ) { @@ -2015,10 +2087,90 @@ wcstring format_size(long long sz) } } - return result; } +/* Crappy function to extract the most significant digit of an unsigned long long value */ +static char extract_most_significant_digit(unsigned long long *xp) { + unsigned long long place_value = 1; + unsigned long long x = *xp; + while (x >= 10) { + x /= 10; + place_value *= 10; + } + *xp -= (place_value * x); + return x + '0'; +} + +void append_ull(char *buff, unsigned long long val, size_t *inout_idx, size_t max_len) { + size_t idx = *inout_idx; + while (val > 0 && idx < max_len) + buff[idx++] = extract_most_significant_digit(&val); + *inout_idx = idx; +} + +void append_str(char *buff, const char *str, size_t *inout_idx, size_t max_len) { + size_t idx = *inout_idx; + while (*str && idx < max_len) + buff[idx++] = *str++; + *inout_idx = idx; +} + +void format_size_safe(char buff[128], unsigned long long sz) { + const size_t buff_size = 128; + const size_t max_len = buff_size - 1; //need to leave room for a null terminator + bzero(buff, buff_size); + size_t idx = 0; + const char * const sz_name[]= { + "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB", NULL + }; + if (sz < 1) + { + strncpy(buff, "empty", buff_size); + } + else if (sz < 1024) + { + append_ull(buff, sz, &idx, max_len); + append_str(buff, "B", &idx, max_len); + } + else + { + for( size_t i=0; sz_name[i]; i++ ) + { + if( sz < (1024*1024) || !sz_name[i+1] ) + { + unsigned long long isz = sz/1024; + if( isz > 9 ) + { + append_ull(buff, isz, &idx, max_len); + } + else + { + if (isz == 0) + { + append_str(buff, "0", &idx, max_len); + } + else + { + append_ull(buff, isz, &idx, max_len); + } + + // Maybe append a single fraction digit + unsigned long long remainder = sz % 1024; + if (remainder > 0) + { + char tmp[3] = {'.', extract_most_significant_digit(&remainder), 0}; + append_str(buff, tmp, &idx, max_len); + } + } + append_str(buff, sz_name[i], &idx, max_len); + break; + } + sz /= 1024; + } + } +} + double timef() { int time_res; @@ -2045,6 +2197,19 @@ void exit_without_destructors(int code) { _exit(code); } +/* Helper function to convert from a null_terminated_array_t to a null_terminated_array_t */ +null_terminated_array_t convert_wide_array_to_narrow(const null_terminated_array_t &wide_arr) { + const wchar_t *const *arr = wide_arr.get(); + if (! arr) + return null_terminated_array_t(); + + std::vector list; + for (size_t i=0; arr[i]; i++) { + list.push_back(wcs2string(arr[i])); + } + return null_terminated_array_t(list); +} + void append_path_component(wcstring &path, const wcstring &component) { size_t len = path.size(); @@ -2076,6 +2241,10 @@ static void child_note_forked(void) { } bool is_forked_child(void) { + if (is_child_of_fork) { + printf("Uh-oh: %d\n", getpid()); + while (1) sleep(10000); + } return is_child_of_fork; } diff --git a/common.h b/common.h index b1186df60..62fa73787 100644 --- a/common.h +++ b/common.h @@ -338,6 +338,7 @@ class null_terminated_array_t { public: null_terminated_array_t() : array(NULL) { } + null_terminated_array_t(const string_list_t &argv) : array(NULL) { this->set(argv); } ~null_terminated_array_t() { this->free(); } /** operator=. Notice the pass-by-value parameter. */ @@ -348,7 +349,7 @@ class null_terminated_array_t { } /* Copy constructor. */ - null_terminated_array_t(const null_terminated_array_t &him) { + null_terminated_array_t(const null_terminated_array_t &him) : array(NULL) { this->set(him.array); } @@ -401,9 +402,11 @@ class null_terminated_array_t { } return lst; } - }; +/* Helper function to convert from a null_terminated_array_t to a null_terminated_array_t */ +null_terminated_array_t convert_wide_array_to_narrow(const null_terminated_array_t &arr); + bool is_forked_child(); /* Basic scoped lock class */ @@ -666,12 +669,20 @@ int create_directory( const wcstring &d ); */ void bugreport(); -/** - Format the specified size (in bytes, kilobytes, etc.) into the specified stringbuffer. -*/ +/** Format the specified size (in bytes, kilobytes, etc.) into the specified stringbuffer. */ void sb_format_size( string_buffer_t *sb, long long sz ); wcstring format_size(long long sz); +/** Version of format_size that does not allocate memory. */ +void format_size_safe(char buff[128], unsigned long long sz); + +/** Our crappier versions of debug which is guaranteed to not allocate any memory, or do anything other than call write(). This is useful after a call to fork() with threads. */ +void debug_safe(int level, const char *msg, const char *param1 = NULL, const char *param2 = NULL, const char *param3 = NULL); + +/** Writes out an int safely */ +void format_int_safe(char buff[128], int val); + + /** Return the number of seconds from the UNIX epoch, with subsecond precision. This function uses the gettimeofday function, and will diff --git a/exec.cpp b/exec.cpp index 9aa58ac65..d7f0e0b60 100644 --- a/exec.cpp +++ b/exec.cpp @@ -193,55 +193,50 @@ void close_unused_internal_pipes( io_data_t *io ) } /** - Returns the interpreter for the specified script. Returns false if file + Returns the interpreter for the specified script. Returns NULL if file is not a script with a shebang. */ -static bool get_interpreter( const wcstring &file, wcstring &interpreter ) +static char *get_interpreter( const char *command, char *interpreter, size_t buff_size ) { - wcstring res; - FILE *fp = wfopen( file, "r" ); - if( fp ) + int fd = open( command, O_RDONLY ); + if( fd >= 0 ) { - while( 1 ) + size_t idx = 0; + while( idx + 1 < buff_size ) { - wint_t ch = getwc( fp ); - if( ch == WEOF ) + char ch; + ssize_t amt = read(fd, &ch, sizeof ch); + if( amt <= 0 ) break; - if( ch == L'\n' ) + if( ch == '\n' ) break; - res.push_back(ch); + interpreter[idx++] = ch; } - fclose(fp); + interpreter[idx++] = '\0'; + close(fd); } - - if (string_prefixes_string(L"#! /", res)) { - interpreter = 3 + res.c_str(); - return true; - } else if (string_prefixes_string(L"#!/", res)) { - interpreter = 2 + res.c_str(); - return true; + if (strncmp(interpreter, "#! /", 4) == 0) { + return interpreter + 3; + } else if (strncmp(interpreter, "#!/", 3) == 0) { + return interpreter + 2; } else { - return false; + return NULL; } } - /** This function is executed by the child process created by a call to fork(). It should be called after \c setup_child_process. It calls execve to replace the fish process image with the command specified in \c p. It never returns. */ -static void launch_process( process_t *p ) +/* Called in a forked child! Do not allocate memory, etc. */ +static void safe_launch_process( process_t *p, const char *actual_cmd, char **argv, char **envv ) { - FILE* f; int err; // debug( 1, L"exec '%ls'", p->argv[0] ); - char **argv = wcsv2strv(p->get_argv()); - char **envv = env_export_arr( 0 ); - execve ( wcs2str(p->actual_cmd), argv, envv ); @@ -253,55 +248,43 @@ static void launch_process( process_t *p ) /bin/sh if encountered. This is a weird predecessor to the shebang that is still sometimes used since it is supported on Windows. */ - f = wfopen(p->actual_cmd, "r"); - if( f ) + int fd = open(actual_cmd, O_RDONLY); + if (fd >= 0) { char begin[1] = {0}; - size_t read; + ssize_t amt_read = read(fd, begin, 1); + close(fd); - read = fread(begin, 1, 1, f); - fclose( f ); - - if( (read==1) && (begin[0] == ':') ) + if( (amt_read==1) && (begin[0] == ':') ) { + // Relaunch it with /bin/sh. Don't allocate memory, so if you have more args than this, update your silly script! Maybe this should be changed to be based on ARG_MAX somehow. + char sh_command[] = "/bin/sh"; + char *argv2[128]; + argv2[0] = sh_command; + for (size_t i=1; i < sizeof argv2 / sizeof *argv2; i++) { + argv2[i] = argv[i-1]; + if (argv2[i] == NULL) + break; + } - wcstring_list_t argv; - - const wchar_t *sh_command = L"/bin/sh"; - argv.push_back(sh_command); - argv.push_back(p->actual_cmd); - for(size_t i=1; p->argv(i) != NULL; i++ ){ - argv.push_back(p->argv(i)); - } - - p->set_argv(argv); - p->actual_cmd = wcsdup(sh_command); - - char **res_real = wcsv2strv( p->get_argv() ); - - execve ( wcs2str(p->actual_cmd), - res_real, - envv ); + execve(sh_command, argv2, envv); } } errno = err; - debug( 0, - _( L"Failed to execute process '%ls'. Reason:" ), - p->actual_cmd ); + debug_safe( 0, "Failed to execute process '%s'. Reason:", actual_cmd ); switch( errno ) { case E2BIG: { - size_t sz = 0; - char **p; - - wcstring sz1, sz2; + char sz1[128], sz2[128]; long arg_max = -1; + size_t sz = 0; + char **p; for(p=argv; *p; p++) { sz += strlen(*p)+1; @@ -312,71 +295,81 @@ static void launch_process( process_t *p ) sz += strlen(*p)+1; } - sz1 = format_size(sz); - + format_size_safe(sz1, sz); arg_max = sysconf( _SC_ARG_MAX ); if( arg_max > 0 ) { - sz2 = format_size(arg_max); - - debug( 0, - L"The total size of the argument and environment lists (%ls) exceeds the operating system limit of %ls.", - sz1.c_str(), - sz2.c_str()); + format_size_safe(sz2, sz); + debug_safe(0, "The total size of the argument and environment lists %s exceeds the operating system limit of %s.", sz1, sz2); } else { - debug( 0, - L"The total size of the argument and environment lists (%ls) exceeds the operating system limit.", - sz1.c_str()); + debug_safe( 0, "The total size of the argument and environment lists (%s) exceeds the operating system limit.", sz1); } - debug( 0, - L"Try running the command again with fewer arguments."); + debug_safe(0, "Try running the command again with fewer arguments."); exit_without_destructors(STATUS_EXEC_FAIL); - break; } case ENOEXEC: { - wperror(L"exec"); + /* Hope strerror doesn't allocate... */ + const char *err = strerror(errno); + debug_safe(0, "exec: %s", err); - debug(0, L"The file '%ls' is marked as an executable but could not be run by the operating system.", p->actual_cmd); + debug_safe(0, "The file '%ls' is marked as an executable but could not be run by the operating system.", actual_cmd); exit_without_destructors(STATUS_EXEC_FAIL); } case ENOENT: { - wcstring interpreter; - if( get_interpreter(p->actual_cmd, interpreter) && waccess( interpreter, X_OK ) ) + char interpreter_buff[128] = {}, *interpreter; + interpreter = get_interpreter(actual_cmd, interpreter_buff, sizeof interpreter_buff); + if( interpreter && 0 != access( interpreter, X_OK ) ) { - debug(0, L"The file '%ls' specified the interpreter '%ls', which is not an executable command.", p->actual_cmd, interpreter.c_str() ); + debug_safe(0, "The file '%s' specified the interpreter '%s', which is not an executable command.", actual_cmd, interpreter ); } else { - debug(0, L"The file '%ls' or a script or ELF interpreter does not exist, or a shared library needed for file or interpreter cannot be found.", p->actual_cmd); + debug_safe(0, "The file '%s' or a script or ELF interpreter does not exist, or a shared library needed for file or interpreter cannot be found.", actual_cmd); } - exit_without_destructors(STATUS_EXEC_FAIL); } case ENOMEM: { - debug(0, L"Out of memory"); + debug_safe(0, "Out of memory"); exit_without_destructors(STATUS_EXEC_FAIL); } default: { - wperror(L"exec"); + /* Hope strerror doesn't allocate... */ + const char *err = strerror(errno); + debug_safe(0, "exec: %s", err); // debug(0, L"The file '%ls' is marked as an executable but could not be run by the operating system.", p->actual_cmd); exit_without_destructors(STATUS_EXEC_FAIL); } } +} + +/** + This function is similar to launch_process, except it is not called after a fork (i.e. it is only calls exec) and therefore it can allocate memory. +*/ +static void launch_process_nofork( process_t *p ) +{ + ASSERT_IS_MAIN_THREAD(); + ASSERT_IS_NOT_FORKED_CHILD(); + + char **argv = wcsv2strv(p->get_argv()); + char **envv = env_export_arr( 0 ); + char *actual_cmd = wcs2str(p->actual_cmd); + /* Bounce to launch_process. This never returns. */ + safe_launch_process(p, actual_cmd, argv, envv); } @@ -630,7 +623,7 @@ void exec( parser_t &parser, job_t *j ) /* launch_process _never_ returns */ - launch_process( j->first_process ); + launch_process_nofork( j->first_process ); } else { @@ -1209,7 +1202,6 @@ void exec( parser_t &parser, job_t *j ) } else { - /* Free the strings in the parent */ free(outbuff); free(errbuff); @@ -1230,6 +1222,18 @@ void exec( parser_t &parser, job_t *j ) case EXTERNAL: { + /* Get argv and envv before we fork */ + null_terminated_array_t argv_array = convert_wide_array_to_narrow(p->get_argv_array()); + + null_terminated_array_t envv_array; + env_export_arr(false, envv_array); + + char **envv = envv_array.get(); + char **argv = argv_array.get(); + + std::string actual_cmd_str = wcs2string(p->actual_cmd); + const char *actual_cmd = actual_cmd_str.c_str(); + pid = execute_fork(true /* must drain threads */); if( pid == 0 ) { @@ -1238,10 +1242,10 @@ void exec( parser_t &parser, job_t *j ) */ p->pid = getpid(); setup_child_process( j, p ); - launch_process( p ); + safe_launch_process( p, actual_cmd, argv, envv ); /* - launch_process _never_ returns... + safe_launch_process _never_ returns... */ } else diff --git a/fish_tests.cpp b/fish_tests.cpp index c8f72e75c..418d09839 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -147,10 +147,32 @@ static void test_escape() free( (void *)e ); free( (void *)u ); - } - + } +} - +static void test_format(void) { + say( L"Testing formatting functions" ); + struct { unsigned long long val; const char *expected; } tests[] = { + { 0, "empty" }, + { 1, "1B" }, + { 2, "2B" }, + { 1024, "1kB" }, + { 1870, "1.8kB" }, + { 4322911, "4.1MB" } + }; + size_t i; + for (i=0; i < sizeof tests / sizeof *tests; i++) { + char buff[128]; + format_size_safe(buff, tests[i].val); + assert( ! strcmp(buff, tests[i].expected)); + } + + for (int j=-129; j <= 129; j++) { + char buff1[128], buff2[128]; + format_int_safe(buff1, j); + sprintf(buff2, "%d", j); + assert( ! strcmp(buff1, buff2)); + } } /** @@ -698,6 +720,7 @@ int main( int argc, char **argv ) reader_init(); env_init(); + test_format(); test_escape(); test_convert(); test_tok(); diff --git a/parser.cpp b/parser.cpp index a92e026cb..6a53936db 100644 --- a/parser.cpp +++ b/parser.cpp @@ -1314,7 +1314,7 @@ void parser_t::parse_job_argument_list( process_t *p, { wchar_t *end; - if( (p->type == INTERNAL_EXEC) ) + if (p->type == INTERNAL_EXEC) { error( SYNTAX_ERROR, tok_get_pos( tok ), @@ -2233,7 +2233,7 @@ void parser_t::skipped_exec( job_t * j ) } else if( wcscmp( p->argv0(), L"case" )==0) { - if( (current_block->type == SWITCH ) ) + if(current_block->type == SWITCH) { exec( *this, j ); return; diff --git a/postfork.cpp b/postfork.cpp index 0be315625..425dc526f 100644 --- a/postfork.cpp +++ b/postfork.cpp @@ -32,19 +32,6 @@ #define FD_ERROR _( L"An error occurred while redirecting file descriptor %d" ) -/** - This function should be called by both the parent process and the - child right after fork() has been called. If job control is - enabled, the child is put in the jobs group, and if the child is - also in the foreground, it is also given control of the - terminal. When called in the parent process, this function may - fail, since the child might have already finished and called - exit. The parent process may safely ignore the exit status of this - call. - - Returns 0 on sucess, -1 on failiure. -*/ - // PCA These calls to debug are rather sketchy because they may allocate memory. Fortunately they only occur if an error occurs. int set_child_group( job_t *j, process_t *p, int print_errors ) { @@ -61,6 +48,7 @@ int set_child_group( job_t *j, process_t *p, int print_errors ) { if( getpgid( p->pid) != j->pgid && print_errors ) { + // PCA FIXME This is sketchy to do in a forked child because it may allocate memory. This needs to call only safe functions. debug( 1, _( L"Could not send process %d, '%ls' in job %d, '%ls' from group %d to group %d" ), p->pid, @@ -284,22 +272,6 @@ static int handle_child_io( io_data_t *io ) } -/** - Initialize a new child process. This should be called right away - after forking in the child process. If job control is enabled for - this job, the process is put in the process group of the job, all - signal handlers are reset, signals are unblocked (this function may - only be called inside the exec function, which blocks all signals), - and all IO redirections and other file descriptor actions are - performed. - - \param j the job to set up the IO for - \param p the child process to set up - - \return 0 on sucess, -1 on failiure. When this function returns, - signals are always unblocked. On failiure, signal handlers, io - redirections and process group of the process is undefined. -*/ int setup_child_process( job_t *j, process_t *p ) { int res=0; diff --git a/postfork.h b/postfork.h index 06364b312..741e6369c 100644 --- a/postfork.h +++ b/postfork.h @@ -19,8 +19,36 @@ #include "wutil.h" #include "io.h" +/** + This function should be called by both the parent process and the + child right after fork() has been called. If job control is + enabled, the child is put in the jobs group, and if the child is + also in the foreground, it is also given control of the + terminal. When called in the parent process, this function may + fail, since the child might have already finished and called + exit. The parent process may safely ignore the exit status of this + call. + Returns 0 on sucess, -1 on failiure. +*/ int set_child_group( job_t *j, process_t *p, int print_errors ); + +/** + Initialize a new child process. This should be called right away + after forking in the child process. If job control is enabled for + this job, the process is put in the process group of the job, all + signal handlers are reset, signals are unblocked (this function may + only be called inside the exec function, which blocks all signals), + and all IO redirections and other file descriptor actions are + performed. + + \param j the job to set up the IO for + \param p the child process to set up + + \return 0 on sucess, -1 on failiure. When this function returns, + signals are always unblocked. On failiure, signal handlers, io + redirections and process group of the process is undefined. +*/ int setup_child_process( job_t *j, process_t *p ); /* Call fork(), optionally waiting until we are no longer multithreaded. If the forked child doesn't do anything that could allocate memory, take a lock, etc. (like call exec), then it's not necessary to wait for threads to die. If the forked child may do those things, it should wait for threads to die. diff --git a/proc.cpp b/proc.cpp index 7546883d8..02b42172d 100644 --- a/proc.cpp +++ b/proc.cpp @@ -335,7 +335,6 @@ int job_signal( job_t *j, int signal ) } return res; - } diff --git a/proc.h b/proc.h index 1a540669b..06757cae6 100644 --- a/proc.h +++ b/proc.h @@ -172,6 +172,7 @@ class process_t /** Returns argv */ const wchar_t * const *get_argv(void) const { return argv_array.get(); } + const null_terminated_array_t &get_argv_array(void) const { return argv_array; } /** Returns argv[0] */ const wchar_t *argv0(void) const { return argv_array.get()[0]; }