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

@@ -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 )
 	{

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
+		   If there is a next process in the pipeline, close the
-		   pipe (the child subprocess already has a copy of the pipe).
+		   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 )
 		{

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 );