From 446272eee6c8059a7cb137e9f3f2e2e1433616db Mon Sep 17 00:00:00 2001 From: axel Date: Sun, 4 Jun 2006 08:35:33 +1000 Subject: [PATCH] Minor code tweaks, including making sure builtins are not run if an io redirection issue occurs and removing a lot commented debug code darcs-hash:20060603223533-ac50b-204ff1fb1a4912565044e7bc4b86102a7eeba192.gz --- builtin.c | 31 +++++++--------- exec.c | 100 +++++++++++++++++++++------------------------------- highlight.c | 5 ++- 3 files changed, 55 insertions(+), 81 deletions(-) diff --git a/builtin.c b/builtin.c index 220f80148..3571493f9 100644 --- a/builtin.c +++ b/builtin.c @@ -1933,7 +1933,7 @@ static int builtin_exit( wchar_t **argv ) /** Helper function for builtin_cd, used for seting the current working directory */ -static int set_pwd(wchar_t *env) +static int set_pwd( wchar_t *env) { wchar_t dir_path[4096]; wchar_t *res = wgetcwd( dir_path, 4096 ); @@ -2003,7 +2003,7 @@ static int builtin_cd( wchar_t **argv ) return 1; } - if (!set_pwd(L"PWD")) + if( !set_pwd(L"PWD") ) { res=1; sb_printf( sb_err, _( L"%ls: Could not set PWD variable\n" ), argv[0] ); @@ -2134,11 +2134,13 @@ static int builtin_fg( wchar_t **argv ) if( argv[1] == 0 ) { /* - Select last constructed job (I.e. first job in the job que) that is possible to put in the foreground + Select last constructed job (I.e. first job in the job que) + that is possible to put in the foreground */ for( j=first_job; j; j=j->next ) { - if( j->constructed && (!job_is_completed(j)) && ( (job_is_stopped(j) || !j->fg) && (j->job_control))) + if( j->constructed && (!job_is_completed(j)) && + ( (job_is_stopped(j) || !j->fg) && (j->job_control) ) ) break; } if( !j ) @@ -2458,7 +2460,7 @@ static int builtin_end( wchar_t **argv ) case SUBST: case BEGIN: /* - Nothing special happens at the end of these. The scope just ends. + Nothing special happens at the end of these commands. The scope just ends. */ break; @@ -2482,10 +2484,6 @@ static int builtin_end( wchar_t **argv ) kill_block = 0; parser_set_pos( current_block->tok_pos ); -/* - fwprintf( stderr, - L"jump to %d\n", - current_block->tok_pos ); */ } break; } @@ -2499,7 +2497,7 @@ static int builtin_end( wchar_t **argv ) */ wchar_t *def = wcsndup( parser_get_buffer()+current_block->tok_pos, parser_get_job_pos()-current_block->tok_pos ); - + function_add( current_block->param1.function_name, def, current_block->param2.function_description, @@ -2632,7 +2630,6 @@ static int builtin_return( wchar_t **argv ) builtin_print_help( argv[0], sb_err ); return 1; } -// fwprintf( stderr, L"Return with status %d\n", status ); break; } default: @@ -2861,6 +2858,10 @@ const static builtin_data_t builtin_data[]= L"ulimit", &builtin_ulimit, N_( L"Set or get the shells resource usage limits" ) } , + { + L"begin", &builtin_begin, N_( L"Create a block of code" ) + } + , /* Builtins that are handled directly by the parser. They are @@ -2895,10 +2896,6 @@ const static builtin_data_t builtin_data[]= L"exec", &builtin_generic, N_( L"Run command in current process" ) } , - { - L"begin", &builtin_begin, N_( L"Create a block of code" ) - } - , /* This is not a builtin, but fish handles it's help display @@ -2995,8 +2992,6 @@ int builtin_run( wchar_t **argv ) int status; status = cmd(argv); -// fwprintf( stderr, L"Builtin: Set status of %ls to %d\n", argv[0], status ); - return status; } @@ -3034,7 +3029,7 @@ const wchar_t *builtin_get_desc( const wchar_t *b ) return _( hash_get( desc, b )); } -void builtin_push_io( int in) +void builtin_push_io( int in ) { if( builtin_stdin != -1 ) { diff --git a/exec.c b/exec.c index 91e66f4bf..7d6b64308 100644 --- a/exec.c +++ b/exec.c @@ -668,7 +668,7 @@ void exec( job_t *j ) signal_block(); /* - setup_child_process make sure signals are properly set + setup_child_process makes sure signals are properly set up. It will also call signal_unblock */ if( !setup_child_process( j, 0 ) ) @@ -757,22 +757,12 @@ void exec( job_t *j ) break; } -/* fwprintf( stderr, - L"Make pipe from %ls to %ls using fds %d and %d\n", - p->actual_cmd, - p->next->actual_cmd, - mypipe[0], - mypipe[1] ); -*/ memcpy( pipe_write.param1.pipe_fd, mypipe, sizeof(int)*2); } else { /* This is the last element of the pipeline. - */ - - /* Remove the io redirection for pipe output. */ j->io = io_remove( j->io, &pipe_write ); @@ -785,7 +775,6 @@ void exec( job_t *j ) { wchar_t * def = halloc_register( j, wcsdup( function_get_definition( p->argv[0] ))); - //fwprintf( stderr, L"run function %ls\n", argv[0] ); if( def == 0 ) { debug( 0, _( L"Unknown function '%ls'" ), p->argv[0] ); @@ -803,7 +792,6 @@ void exec( job_t *j ) if( p->next ) { -// fwprintf( stderr, L"Function %ls\n", def ); io_buffer = io_buffer_create(); j->io = io_add( j->io, io_buffer ); } @@ -820,7 +808,6 @@ void exec( job_t *j ) { if( p->next ) { -// fwprintf( stderr, L"Block %ls\n", p->argv[0] ); io_buffer = io_buffer_create(); j->io = io_add( j->io, io_buffer ); } @@ -848,11 +835,6 @@ void exec( job_t *j ) case IO_FD: { builtin_stdin = in->param1.old_fd; -/* fwprintf( stderr, - L"Input redirection for builtin '%ls' from fd %d\n", - p->argv[0], - in->old_fd ); -*/ break; } case IO_PIPE: @@ -863,11 +845,6 @@ void exec( job_t *j ) case IO_FILE: { -/* - fwprintf( stderr, - L"Input redirection for builtin from file %ls\n", - in->filename); -*/ builtin_stdin=wopen( in->param1.filename, in->param2.flags, 0777 ); if( builtin_stdin == -1 ) @@ -907,40 +884,37 @@ void exec( job_t *j ) exec_error=1; break; } + else + { + builtin_push_io( builtin_stdin ); + + /* + Since this may be the foreground job, and since a + builtin may execute another foreground job, we need to + pretend to suspend this job while running the builtin. + */ + + builtin_out_redirect = has_fd( j->io, 1 ); + builtin_err_redirect = has_fd( j->io, 2 ); + fg = j->fg; + j->fg = 0; + + signal_unblock(); + + p->status = builtin_run( p->argv ); + + signal_block(); + + /* + Restore the fg flag, which is temporarily set to + false during builtin execution so as not to confuse + some job-handling builtins. + */ + j->fg = fg; + } - builtin_push_io( builtin_stdin ); - - /* - Since this may be the foreground job, and since a - builtin may execute another foreground job, we need to - pretend to suspend this job while running the builtin. - */ - - builtin_out_redirect = has_fd( j->io, 1 ); - builtin_err_redirect = has_fd( j->io, 2 ); - fg = j->fg; - j->fg = 0; - - signal_unblock(); - - p->status = builtin_run( p->argv ); - - signal_block(); - - /* - Restore the fg flag, which is temporarily set to - false during builtin execution so as not to confuse - some job-handling builtins. - */ - j->fg = fg; - - -/* if( is_interactive ) - fwprintf( stderr, L"Builtin '%ls' finished\n", j->command ); -*/ if( close_stdin ) { -// fwprintf( stderr, L"Close builtin_stdin\n" ); exec_close( builtin_stdin ); } break; @@ -1059,7 +1033,6 @@ void exec( job_t *j ) ( sb_out->used ) && ( buffer_stdout ) ) { -// fwprintf( stderr, L"Skip fork of %ls\n", j->command ); char *res = wcs2str( (wchar_t *)sb_out->buff ); b_append( io->param2.out_buffer, res, strlen( res ) ); skip_fork = 1; @@ -1082,9 +1055,13 @@ void exec( job_t *j ) pid = fork(); if( pid == 0 ) { + /* - This is the child process. + This is the child process. Setup redirections, + print correct output to stdout and stderr, and + then exit. */ + p->pid = getpid(); setup_child_process( j, p ); if( sb_out->used ) @@ -1106,7 +1083,7 @@ void exec( job_t *j ) { /* This is the parent process. Store away - information on the child, and possibly fice + information on the child, and possibly give it control over the terminal. */ p->pid = pid; @@ -1173,8 +1150,11 @@ void exec( job_t *j ) pipe_read.param1.pipe_fd[0] = mypipe[0]; /* - If there is a next process, close the output end of the - pipe (the child subprocess already has a copy of the pipe). + If there is a next process in the pipeline, close the + output end of the current pipe (the surrent child + subprocess already has a copy of the pipe - this makes sure + we don't leak file descriptors either in the shell or in + the children). */ if( p->next ) { diff --git a/highlight.c b/highlight.c index a785ab24b..2966bc2b6 100644 --- a/highlight.c +++ b/highlight.c @@ -814,8 +814,7 @@ static void highlight_universal_internal( wchar_t * buff, wchar_t inc_char = buff[pos]; int level = 0; wchar_t *str = &buff[pos]; - int match_found=0; - + int match_found=0; while( (str >= buff) && *str) { @@ -847,7 +846,7 @@ void highlight_universal( wchar_t * buff, { int i; - for( i=0; buff[i] != 0; i++ ) + for( i=0; buff[i]; i++ ) color[i] = 0; highlight_universal_internal( buff, color, pos, error );