diff --git a/exec.cpp b/exec.cpp index fd4bedc14..b3195fcef 100644 --- a/exec.cpp +++ b/exec.cpp @@ -420,7 +420,7 @@ static io_data_t *io_transmogrify( io_data_t * in ) if( !in ) return 0; - std::auto_ptr out(new io_data_t()); + std::auto_ptr out(new io_data_t); out->fd = in->fd; out->io_mode = IO_FD; @@ -437,7 +437,7 @@ static io_data_t *io_transmogrify( io_data_t * in ) case IO_BUFFER: case IO_PIPE: { - *out = *in; + out.reset(new io_data_t(*in)); break; } @@ -1039,7 +1039,10 @@ void exec( parser_t &parser, job_t *j ) io_buffer_read( io_buffer ); - if( io_buffer->out_buffer->used != 0 ) + const char *buffer = io_buffer->out_buffer_ptr(); + size_t count = io_buffer->out_buffer_size(); + + if( io_buffer->out_buffer_size() > 0 ) { /* We don't have to drain threads here because our child process is simple */ pid = execute_fork(false); @@ -1052,10 +1055,7 @@ void exec( parser_t &parser, job_t *j ) p->pid = getpid(); setup_child_process( j, p ); - exec_write_and_exit(io_buffer->fd, - io_buffer->out_buffer->buff, - io_buffer->out_buffer->used, - status); + exec_write_and_exit(io_buffer->fd, buffer, count, status); } else { @@ -1090,8 +1090,10 @@ void exec( parser_t &parser, job_t *j ) case INTERNAL_BUFFER: { + const char *buffer = io_buffer->out_buffer_ptr(); + size_t count = io_buffer->out_buffer_size(); + pid = execute_fork(false); - if( pid == 0 ) { /* @@ -1101,10 +1103,7 @@ void exec( parser_t &parser, job_t *j ) p->pid = getpid(); setup_child_process( j, p ); - exec_write_and_exit( 1, - input_redirect->out_buffer->buff, - input_redirect->out_buffer->used, - 0); + exec_write_and_exit( 1, buffer, count, 0); } else { @@ -1157,7 +1156,7 @@ void exec( parser_t &parser, job_t *j ) ( buffer_stdout ) ) { std::string res = wcs2string( get_stdout_buffer() ); - b_append( io->out_buffer, res.c_str(), res.size() ); + io->out_buffer_append( res.c_str(), res.size() ); skip_fork = 1; } @@ -1378,9 +1377,9 @@ static int exec_subshell_internal( const wcstring &cmd, wcstring_list_t *lst ) is_subshell = prev_subshell; - b_append( io_buffer->out_buffer, &z, 1 ); + io_buffer->out_buffer_append( &z, 1 ); - begin=end=io_buffer->out_buffer->buff; + begin=end=io_buffer->out_buffer_ptr(); if( lst ) { diff --git a/history.h b/history.h index 27b1c10cb..4e825fdb7 100644 --- a/history.h +++ b/history.h @@ -14,7 +14,6 @@ #include #include #include -using std::tr1::shared_ptr; typedef std::list path_list_t; diff --git a/io.cpp b/io.cpp index 91f3f8c3b..08bdf5d56 100644 --- a/io.cpp +++ b/io.cpp @@ -91,7 +91,7 @@ void io_buffer_read( io_data_t *d ) } else { - b_append( d->out_buffer, b, l ); + d->out_buffer_append( b, l ); } } } @@ -100,21 +100,18 @@ void io_buffer_read( io_data_t *d ) io_data_t *io_buffer_create( int is_input ) { - std::auto_ptr buffer_redirect(new io_data_t()); - + std::auto_ptr buffer_redirect(new io_data_t); + buffer_redirect->out_buffer_create(); buffer_redirect->io_mode=IO_BUFFER; buffer_redirect->next=0; - buffer_redirect->out_buffer= (buffer_t *)malloc( sizeof(buffer_t)); buffer_redirect->is_input = is_input; - b_init( buffer_redirect->out_buffer ); buffer_redirect->fd=is_input?0:1; if( exec_pipe( buffer_redirect->param1.pipe_fd ) == -1 ) { debug( 1, PIPE_ERROR ); wperror (L"pipe"); - free( buffer_redirect->out_buffer ); - return 0; + return NULL; } else if( fcntl( buffer_redirect->param1.pipe_fd[0], F_SETFL, @@ -122,8 +119,7 @@ io_data_t *io_buffer_create( int is_input ) { debug( 1, PIPE_ERROR ); wperror( L"fcntl" ); - free( buffer_redirect->out_buffer ); - return 0; + return NULL; } return buffer_redirect.release(); } @@ -146,10 +142,6 @@ void io_buffer_destroy( io_data_t *io_buffer ) Dont free fd for writing. This should already be free'd before calling exec_read_io_buffer on the buffer */ - - b_destroy( io_buffer->out_buffer ); - - free( io_buffer->out_buffer ); delete io_buffer; } diff --git a/io.h b/io.h index 4ef3acc97..bd18ed41f 100644 --- a/io.h +++ b/io.h @@ -1,22 +1,33 @@ #ifndef FISH_IO_H #define FISH_IO_H +#include +using std::tr1::shared_ptr; + /** Describes what type of IO operation an io_data_t represents */ enum io_mode { IO_FILE, IO_PIPE, IO_FD, IO_BUFFER, IO_CLOSE -} -; +}; /** Represents an FD redirection */ -struct io_data_t +class io_data_t { +private: + /** buffer to save output in for IO_BUFFER. Note that in the original fish, the buffer was a pointer to a buffer_t stored in the param2 union down below, and when an io_data_t was duplicated the pointer was copied so that two io_data_ts referenced the same buffer. It's not clear to me how this was ever cleaned up correctly. But it's important that they share the same buffer for reasons I don't yet understand either. But we can get correct sharing and cleanup with shared_ptr. */ + shared_ptr > out_buffer; + + /* No assignment allowed */ + void operator=(const io_data_t &rhs) { assert(0); } + +public: /** Type of redirect */ int io_mode; /** FD to redirect */ int fd; + /** Type-specific parameter for redirection */ @@ -31,9 +42,7 @@ struct io_data_t /** Filename IO_FILE */ wcstring filename; - /** - Second type-specific paramter for redirection - */ + /** Second type-specific paramter for redirection */ union { /** file creation flags to send to open for IO_FILE */ @@ -42,8 +51,28 @@ struct io_data_t int close_old; } param2; - /** buffer to save output in for IO_BUFFER */ - buffer_t *out_buffer; + /** Function to create the output buffer */ + void out_buffer_create() { + out_buffer.reset(new std::vector); + } + + /** Function to append to the buffer */ + void out_buffer_append(const char *ptr, size_t count) { + assert(out_buffer.get() != NULL); + out_buffer->insert(out_buffer->end(), ptr, ptr + count); + } + + /** Function to get a pointer to the buffer */ + char *out_buffer_ptr(void) { + assert(out_buffer.get() != NULL); + return &out_buffer->at(0); + } + + /** Function to get the size of the buffer */ + size_t out_buffer_size(void) const { + assert(out_buffer.get() != NULL); + return out_buffer->size(); + } /** Set to true if this is an input io redirection */ int is_input; @@ -51,7 +80,11 @@ struct io_data_t /** Pointer to the next IO redirection */ io_data_t *next; - io_data_t() : next(NULL) { } + io_data_t() : next(NULL) + { + } + + /* Note: we have a default copy constructor */ }; diff --git a/postfork.cpp b/postfork.cpp index d6da00a6d..1bb796280 100644 --- a/postfork.cpp +++ b/postfork.cpp @@ -352,4 +352,5 @@ pid_t execute_fork(bool wait_for_threads_to_die) debug( 0, FORK_ERROR ); wperror (L"fork"); FATAL_EXIT(); + return 0; } diff --git a/proc.cpp b/proc.cpp index 109a46220..fe0f5bb31 100644 --- a/proc.cpp +++ b/proc.cpp @@ -881,7 +881,7 @@ static void read_try( job_t *j ) } else { - b_append( buff->out_buffer, b, l ); + buff->out_buffer_append(b, l); } } } diff --git a/reader.cpp b/reader.cpp index 261b82f08..ba62b1b79 100644 --- a/reader.cpp +++ b/reader.cpp @@ -1174,7 +1174,7 @@ static void run_pager( wchar_t *prefix, int is_quoted, const std::vectorout_buffer, foo, strlen(foo) ); + in->out_buffer_append(foo, strlen(foo) ); free( foo ); term_donate(); @@ -1190,10 +1190,10 @@ static void run_pager( wchar_t *prefix, int is_quoted, const std::vectorout_buffer, &nil, 1 ); + out->out_buffer_append((char *)&nil, 1); wchar_t *tmp; - wchar_t *str = str2wcs((char *)out->out_buffer->buff); + wchar_t *str = str2wcs(out->out_buffer_ptr()); if( str ) { diff --git a/wutil.cpp b/wutil.cpp index 30a840442..7e4289316 100644 --- a/wutil.cpp +++ b/wutil.cpp @@ -66,8 +66,6 @@ void wutil_destroy() { } -static pthread_mutex_t readdir_lock = PTHREAD_MUTEX_INITIALIZER; - bool wreaddir_resolving(DIR *dir, const std::wstring &dir_path, std::wstring &out_name, bool *out_is_dir) { struct dirent *d = readdir( dir );