Fix lots of bugs related to the static analyzer

Improved how screen.cpp interacts with output_set_writer()
This commit is contained in:
ridiculousfish 2012-03-26 01:21:10 -07:00
parent 31b7d076b7
commit 0bc644abf0
24 changed files with 106 additions and 182 deletions

View file

@ -2723,6 +2723,7 @@ static int builtin_contains( parser_t &parser, wchar_t ** argv )
switch( opt )
{
case 0:
assert(opt_index >= 0 && opt_index < sizeof long_options / sizeof *long_options);
if(long_options[opt_index].flag != 0)
break;
append_format(stderr_buffer,

View file

@ -232,17 +232,6 @@ void rgb_color_t::parse(const wcstring &str) {
bzero(this->data.rgb, sizeof this->data.rgb);
this->type = type_none;
}
if (! success) success = try_parse_special(str);
if (this->try_parse_special(str)) {
/* Nothing */
} else if (this->try_parse_named(str)) {
} else if (this->try_parse_rgb(str)) {
this->type = type_rgb;
} else {
bzero(this->data.rgb, sizeof this->data.rgb);
this->type = type_none;
}
}
rgb_color_t::rgb_color_t(const wcstring &str) {

View file

@ -77,8 +77,8 @@ typedef std::vector<wcstring> wcstring_list_t;
*/
#define VOMIT_ON_FAILURE(a) do { if (0 != (a)) { int err = errno; fprintf(stderr, "%s failed on line %d in file %s: %d (%s)\n", #a, __LINE__, __FILE__, err, strerror(err)); abort(); }} while (0)
/** Exits without invoking destructors (via _exit), useful for code after fork. This would be a good candidate for __noreturn attribute. */
void exit_without_destructors(int code);
/** Exits without invoking destructors (via _exit), useful for code after fork. */
void exit_without_destructors(int code) __attribute__ ((noreturn));
/**
Save the shell mode on startup so we can restore them on exit

View file

@ -563,7 +563,7 @@ int complete_is_valid_option( const wchar_t *str,
int is_old_opt=0;
int is_short_opt=0;
int is_gnu_exact=0;
int gnu_opt_len=0;
size_t gnu_opt_len=0;
std::vector<char> short_validated;
@ -653,12 +653,12 @@ int complete_is_valid_option( const wchar_t *str,
for (option_list_t::const_iterator iter = options.begin(); iter != options.end(); ++iter)
{
const complete_entry_opt_t &o = *iter;
if( o.old_mode )
if (o.old_mode )
{
continue;
}
if( &opt[2] == o.long_opt )
if (wcsncmp(&opt[2], o.long_opt.c_str(), gnu_opt_len) == 0)
{
gnu_match_set.insert(o.long_opt);
if( (wcsncmp( &opt[2],
@ -1535,12 +1535,10 @@ bool completer_t::complete_variable(const wcstring &str, int start_offset)
{
wcstring comp;
int flags = 0;
int offset = 0;
if( match )
{
comp.append(env_name.c_str() + varlen);
offset = varlen;
}
else
{

19
env.cpp
View file

@ -708,15 +708,12 @@ void env_destroy()
*/
static env_node_t *env_get_node( const wcstring &key )
{
var_entry_t* res = NULL;
env_node_t *env = top;
while( env != 0 )
while( env != NULL )
{
if ( env->env.find( key ) != env->env.end() )
{
return env;
break;
}
if( env->new_scope )
@ -728,8 +725,7 @@ static env_node_t *env_get_node( const wcstring &key )
env = env->next;
}
}
return 0;
return env;
}
int env_set(const wchar_t *key, const wchar_t *val, int var_mode)
@ -817,14 +813,9 @@ int env_set(const wchar_t *key, const wchar_t *val, int var_mode)
node = env_get_node( key );
if( node )
{
var_table_t::iterator result = node->env.find(key);
if ( result != node->env.end() ) {
e = result->second;
}
else {
e = NULL;
}
assert(result != node->env.end());
e = result->second;
if( e->exportv )
{

View file

@ -545,28 +545,21 @@ static int match( const wchar_t *msg, const wchar_t *cmd )
void env_universal_common_set( const wchar_t *key, const wchar_t *val, int exportv )
{
var_uni_entry_t *entry;
wchar_t *name;
CHECK( key, );
CHECK( val, );
entry = new var_uni_entry_t;
name = wcsdup(key);
if( !entry || !name )
DIE_MEM();
entry->exportv=exportv;
entry->val = val;
env_universal_common_remove( name );
env_universal_common_remove( key );
env_universal_var.insert( std::pair<wcstring, var_uni_entry_t*>(key, entry));
if( callback )
{
callback( exportv?SET_EXPORT:SET, name, val );
callback( exportv?SET_EXPORT:SET, key, val );
}
free(name);
}

View file

@ -205,7 +205,7 @@ wcstring event_get_desc( const event_t *e )
if( j )
result = format_string(_(L"exit handler for job %d, '%ls'"), j->job_id, j->command_wcstr() );
else
result = format_string(_(L"exit handler for job with job id %d"), j->job_id );
result = format_string(_(L"exit handler for job with job id %d"), e->param1.job_id );
break;
}

View file

@ -394,6 +394,7 @@ static void io_untransmogrify( io_data_t * in, io_data_t *out )
{
if( !out )
return;
assert(in != NULL);
io_untransmogrify( in->next, out->next );
switch( in->io_mode )
{
@ -548,7 +549,6 @@ void exec( parser_t &parser, job_t *j )
pid_t pid;
int mypipe[2];
sigset_t chldset;
int skip_fork;
io_data_t pipe_read, pipe_write;
io_data_t *tmp;
@ -720,7 +720,6 @@ void exec( parser_t &parser, job_t *j )
{
const bool p_wants_pipe = (p->next != NULL);
mypipe[1]=-1;
skip_fork=0;
pipe_write.fd = p->pipe_write_fd;
pipe_read.fd = p->pipe_read_fd;

View file

@ -801,7 +801,6 @@ static int expand_variables2( parser_t &parser, const wcstring &instr, std::vect
static int expand_variables_internal( parser_t &parser, wchar_t * const in, std::vector<completion_t> &out, int last_idx )
{
wchar_t prev_char=0;
int is_ok= 1;
int empty=0;
@ -819,7 +818,6 @@ static int expand_variables_internal( parser_t &parser, wchar_t * const in, std:
int stop_pos;
int var_len;
int is_single = (c==VARIABLE_EXPAND_SINGLE);
int var_name_stop_pos;
stop_pos = start_pos;
@ -833,7 +831,6 @@ static int expand_variables_internal( parser_t &parser, wchar_t * const in, std:
stop_pos++;
}
var_name_stop_pos = stop_pos;
/* printf( "Stop for '%c'\n", in[stop_pos]);*/
@ -1000,8 +997,6 @@ static int expand_variables_internal( parser_t &parser, wchar_t * const in, std:
}
prev_char = c;
}
if( !empty )
@ -1151,7 +1146,6 @@ static int expand_cmdsubst( parser_t &parser, const wcstring &input, std::vector
{
wchar_t *paran_begin=0, *paran_end=0;
int len1;
wchar_t prev=0;
std::vector<wcstring> sub_res;
size_t i, j;
wchar_t *tail_begin = 0;
@ -1178,7 +1172,6 @@ static int expand_cmdsubst( parser_t &parser, const wcstring &input, std::vector
}
len1 = (paran_begin-in);
prev=0;
const wcstring subcmd(paran_begin + 1, paran_end-paran_begin - 1);

View file

@ -660,8 +660,7 @@ static wchar_t *fishd_env_get( const wchar_t *key )
res = env_universal_common_get( key );
if( res )
res = wcsdup( res );
return env_universal_common_get( key );
return res;
}
}

View file

@ -292,7 +292,6 @@ static void highlight_param( const wcstring &buffstr, std::vector<int> &colors,
int chars=2;
int base=16;
int byte = 0;
wchar_t max_val = ASCII_MAX;
switch( buff[in_pos] )
@ -318,7 +317,6 @@ static void highlight_param( const wcstring &buffstr, std::vector<int> &colors,
case L'X':
{
byte=1;
max_val = BYTE_MAX;
break;
}

View file

@ -508,13 +508,13 @@ void history_t::populate_from_mmap(void)
}
}
void history_t::load_old_if_needed(void)
bool history_t::load_old_if_needed(void)
{
if (loaded_old) return;
if (loaded_old) return true;
loaded_old = true;
int fd;
int ok=0;
bool ok=false;
// PCA not sure why signals were blocked here
//signal_block();
@ -532,7 +532,7 @@ void history_t::load_old_if_needed(void)
{
if( (mmap_start = (char *)mmap( 0, mmap_length, PROT_READ, MAP_PRIVATE, fd, 0 )) != MAP_FAILED )
{
ok = 1;
ok = true;
time_profiler_t profiler("populate_from_mmap");
this->populate_from_mmap();
}
@ -542,6 +542,7 @@ void history_t::load_old_if_needed(void)
}
}
//signal_unblock();
return ok;
}
void history_search_t::skip_matches(const wcstring_list_t &skips) {

View file

@ -123,7 +123,7 @@ private:
bool loaded_old;
/** Loads old if necessary */
void load_old_if_needed(void);
bool load_old_if_needed(void);
/** Deletes duplicates in new_items. */
void compact_new_items();

View file

@ -72,8 +72,9 @@ void input_common_destroy()
*/
static wint_t readb()
{
/* do_loop must be set on every path through the loop; leaving it uninitialized allows the static analyzer to assist in catching mistakes. */
unsigned char arr[1];
bool do_loop = false;
bool do_loop;
do
{
@ -98,9 +99,6 @@ static wint_t readb()
if (fd_max < ioport) fd_max = ioport;
}
do_loop = false;
res = select( fd_max + 1, &fdset, 0, 0, 0 );
if( res==-1 )
{
@ -138,39 +136,37 @@ static wint_t readb()
}
else
{
if( env_universal_server.fd > 0 )
{
if( FD_ISSET( env_universal_server.fd, &fdset ) )
{
debug( 3, L"Wake up on universal variable event" );
env_universal_read_all();
do_loop = true;
/* Assume we loop unless we see a character in stdin */
do_loop = true;
if( lookahead_count )
{
return lookahead_arr[--lookahead_count];
}
}
if( env_universal_server.fd > 0 && FD_ISSET( env_universal_server.fd, &fdset ) )
{
debug( 3, L"Wake up on universal variable event" );
env_universal_read_all();
if( lookahead_count )
{
return lookahead_arr[--lookahead_count];
}
}
if ( ioport > 0 && FD_ISSET(ioport, &fdset))
{
iothread_service_completion();
if( lookahead_count )
{
return lookahead_arr[--lookahead_count];
}
}
if ( ioport > 0 )
{
if (FD_ISSET(ioport, &fdset) )
{
iothread_service_completion();
}
do_loop = true;
}
if( FD_ISSET( 0, &fdset ) )
if( FD_ISSET( STDIN_FILENO, &fdset ) )
{
if( read_blocked( 0, arr, 1 ) != 1 )
{
/*
The teminal has been closed. Save and exit.
*/
/* The teminal has been closed. Save and exit. */
return R_EOF;
}
/* We read from stdin, so don't loop */
do_loop = false;
}
}

View file

@ -202,9 +202,9 @@ void iothread_drain_all(void) {
ASSERT_IS_NOT_FORKED_CHILD();
if (s_active_thread_count == 0)
return;
int thread_count = s_active_thread_count;
#define TIME_DRAIN 0
#if TIME_DRAIN
int thread_count = s_active_thread_count;
double now = timef();
#endif
while (s_active_thread_count > 0) {

View file

@ -614,6 +614,7 @@ static char *get_description( const char *mimetype )
{
perror( "read" );
error=1;
free((void *)contents);
return 0;
}

View file

@ -489,14 +489,12 @@ void parse_util_token_extent( const wchar_t *buff,
tokenizer tok;
const wchar_t *a, *b, *pa, *pb;
const wchar_t *a = NULL, *b = NULL, *pa = NULL, *pb = NULL;
CHECK( buff, );
assert( cursor_pos >= 0 );
a = b = pa = pb = 0;
parse_util_cmdsubst_extent( buff, cursor_pos, &begin, &end );
if( !end || !begin )

View file

@ -2260,8 +2260,9 @@ void parser_t::eval_job( tokenizer *tok )
profile_item_t *profile_item = NULL;
int skip = 0;
int job_begin_pos, prev_tokenizer_pos;
const bool do_profile = profile;
if( profile )
if( do_profile )
{
profile_items.resize(profile_items.size() + 1);
profile_item = &profile_items.back();
@ -2316,7 +2317,7 @@ void parser_t::eval_job( tokenizer *tok )
else
j->set_command(L"");
if( profile )
if( do_profile )
{
t2 = get_time();
profile_item->cmd = wcsdup( j->command_wcstr() );
@ -2346,7 +2347,7 @@ void parser_t::eval_job( tokenizer *tok )
this->skipped_exec( j );
}
if( profile )
if( do_profile )
{
t3 = get_time();
profile_item->level=eval_level;
@ -2511,8 +2512,6 @@ int parser_t::eval( const wcstring &cmdStr, io_data_t *io, enum block_type_t blo
event_fire( NULL );
}
int prev_block_type = current_block->type;
parser_t::pop_block();
while( start_current_block != current_block )
@ -2543,7 +2542,6 @@ int parser_t::eval( const wcstring &cmdStr, io_data_t *io, enum block_type_t blo
break;
}
prev_block_type = current_block->type;
parser_t::pop_block();
}
@ -2642,7 +2640,7 @@ int parser_t::parser_test_argument( const wchar_t *arg, wcstring *out, const wch
this->print_errors( *out, prefix);
}
free( arg_cpy );
return 1;
return err;
case 0:
do_loop = 0;

View file

@ -185,7 +185,6 @@ wchar_t *path_get_path( const wchar_t *cmd )
its arguments
*/
wchar_t *path_cpy = wcsdup( path.c_str() );
const wchar_t *nxt_path = path.c_str();
wchar_t *state;
if( (new_cmd==0) || (path_cpy==0) )
@ -193,7 +192,7 @@ wchar_t *path_get_path( const wchar_t *cmd )
DIE_MEM();
}
for( nxt_path = wcstok( path_cpy, ARRAY_SEP_STR, &state );
for( const wchar_t *nxt_path = wcstok( path_cpy, ARRAY_SEP_STR, &state );
nxt_path != 0;
nxt_path = wcstok( 0, ARRAY_SEP_STR, &state) )
{
@ -366,17 +365,15 @@ wchar_t *path_allocate_cdpath( const wchar_t *dir, const wchar_t *wd )
paths.push_back(path);
} else {
wchar_t *path_cpy;
const wchar_t *nxt_path;
wchar_t *state;
wchar_t *whole_path;
env_var_t path = L".";
nxt_path = path.c_str();
path_cpy = wcsdup( path.c_str() );
for( nxt_path = wcstok( path_cpy, ARRAY_SEP_STR, &state );
nxt_path != 0;
for( const wchar_t *nxt_path = wcstok( path_cpy, ARRAY_SEP_STR, &state );
nxt_path != NULL;
nxt_path = wcstok( 0, ARRAY_SEP_STR, &state) )
{
wchar_t *expanded_path = expand_tilde_compat( wcsdup(nxt_path) );

View file

@ -283,7 +283,7 @@ int job_is_stopped( const job_t *j )
int job_is_completed( const job_t *j )
{
process_t *p;
assert(j->first_process != NULL);
for (p = j->first_process; p->next; p = p->next)
;

View file

@ -545,17 +545,7 @@ void reader_data_t::command_line_changed() {
}
/**
Sort an array_list_t containing compltion_t structs.
*/
static void sort_completion_list( std::vector<completion_t> &comp ) {
sort(comp.begin(), comp.end());
}
/**
Remove any duplicate completions in the list. This relies on the
list first beeing sorted.
*/
/** Remove any duplicate completions in the list. This relies on the list first beeing sorted. */
static void remove_duplicates(std::vector<completion_t> &l) {
l.erase(std::unique( l.begin(), l.end()), l.end());
@ -947,6 +937,8 @@ static void get_param( const wchar_t *cmd,
*/
static void completion_insert( const wchar_t *val, int flags )
{
assert(data != NULL);
wchar_t *replaced;
wchar_t quote;
@ -959,7 +951,7 @@ static void completion_insert( const wchar_t *val, int flags )
if( do_replace )
{
int tok_start, tok_len, move_cursor;
int move_cursor;
const wchar_t *begin, *end;
wchar_t *escaped;
@ -967,9 +959,6 @@ static void completion_insert( const wchar_t *val, int flags )
parse_util_token_extent( buff, data->buff_pos, &begin, 0, 0, 0 );
end = buff + data->buff_pos;
tok_start = begin - buff;
tok_len = end-begin;
wcstring sb(buff, begin - buff);
if( do_escape )
@ -1888,6 +1877,10 @@ static void reset_token_history()
*/
static void handle_token_history( int forward, int reset )
{
/* Paranoia */
if (! data)
return;
const wchar_t *str=0;
int current_pos;
tokenizer tok;
@ -2723,8 +2716,6 @@ const wchar_t *reader_readline()
while( !finished && !data->end_loop)
{
int regular_char = 0;
/*
Sometimes strange input sequences seem to generate a zero
byte. I believe these simply mean a character was pressed
@ -2865,7 +2856,6 @@ const wchar_t *reader_readline()
const wchar_t *begin, *end;
const wchar_t *token_begin, *token_end;
const wchar_t *buff = data->command_line.c_str();
wchar_t *buffcpy;
int len;
int cursor_steps;
@ -2883,20 +2873,15 @@ const wchar_t *reader_readline()
reader_repaint();
len = data->buff_pos - (begin-buff);
buffcpy = wcsndup( begin, len );
const wcstring buffcpy = wcstring(begin, len);
// comp = al_halloc( 0 );
data->complete_func( buffcpy, comp, COMPLETE_DEFAULT, NULL);
sort_completion_list( comp );
sort(comp.begin(), comp.end());
remove_duplicates( comp );
free( buffcpy );
comp_empty = handle_completions( comp );
comp.clear();
// halloc_free( comp );
// comp = 0;
}
break;
@ -3319,7 +3304,7 @@ const wchar_t *reader_readline()
if( (!wchar_private(c)) && (( (c>31) || (c==L'\n'))&& (c != 127)) )
{
regular_char = 1;
/* Regular character */
insert_char( c );
}
else

View file

@ -67,6 +67,27 @@ efficient way for transforming that to the desired screen content.
typedef std::vector<char> data_buffer_t;
static data_buffer_t *s_writeb_buffer=0;
static int s_writeb( char c );
/* Class to temporarily set s_writeb_buffer and the writer function in a scoped way */
class scoped_buffer_t {
data_buffer_t * const old_buff;
int (* const old_writer)(char);
public:
scoped_buffer_t(data_buffer_t *buff) : old_buff(s_writeb_buffer), old_writer(output_get_writer())
{
s_writeb_buffer = buff;
output_set_writer(s_writeb);
}
~scoped_buffer_t()
{
s_writeb_buffer = old_buff;
output_set_writer(old_writer);
}
};
/**
Tests if the specified narrow character sequence is present at the
specified position of the specified wide character string. All of
@ -449,16 +470,13 @@ static void s_move( screen_t *s, data_buffer_t *b, int new_x, int new_y )
int i;
int x_steps, y_steps;
int (*writer_old)(char) = output_get_writer();
char *str;
/*
debug( 0, L"move from %d %d to %d %d",
s->screen_cursor[0], s->screen_cursor[1],
new_x, new_y );
*/
output_set_writer( &s_writeb );
s_writeb_buffer = b;
scoped_buffer_t scoped_buffer(b);
y_steps = new_y - s->actual.cursor[1];
@ -514,9 +532,6 @@ static void s_move( screen_t *s, data_buffer_t *b, int new_x, int new_y )
s->actual.cursor[0] = new_x;
s->actual.cursor[1] = new_y;
output_set_writer( writer_old );
}
/**
@ -524,18 +539,11 @@ static void s_move( screen_t *s, data_buffer_t *b, int new_x, int new_y )
*/
static void s_set_color( screen_t *s, data_buffer_t *b, int c )
{
int (*writer_old)(char) = output_get_writer();
output_set_writer( &s_writeb );
s_writeb_buffer = b;
scoped_buffer_t scoped_buffer(b);
unsigned int uc = (unsigned int)c;
set_color( highlight_get_color( uc & 0xffff, false ),
highlight_get_color( (uc>>16)&0xffff, true ) );
output_set_writer( writer_old );
}
/**
@ -544,15 +552,9 @@ static void s_set_color( screen_t *s, data_buffer_t *b, int c )
*/
static void s_write_char( screen_t *s, data_buffer_t *b, wchar_t c )
{
int (*writer_old)(char) = output_get_writer();
output_set_writer( &s_writeb );
s_writeb_buffer = b;
scoped_buffer_t scoped_buffer(b);
s->actual.cursor[0]+=wcwidth( c );
writech( c );
output_set_writer( writer_old );
}
/**
@ -561,14 +563,8 @@ static void s_write_char( screen_t *s, data_buffer_t *b, wchar_t c )
*/
static void s_write_mbs( data_buffer_t *b, char *s )
{
int (*writer_old)(char) = output_get_writer();
output_set_writer( &s_writeb );
s_writeb_buffer = b;
scoped_buffer_t scoped_buffer(b);
writembs( s );
output_set_writer( writer_old );
}
/**
@ -577,14 +573,8 @@ static void s_write_mbs( data_buffer_t *b, char *s )
*/
static void s_write_str( data_buffer_t *b, const wchar_t *s )
{
int (*writer_old)(char) = output_get_writer();
output_set_writer( &s_writeb );
s_writeb_buffer = b;
scoped_buffer_t scoped_buffer(b);
writestr( s );
output_set_writer( writer_old );
}
/**

View file

@ -212,7 +212,6 @@ static void read_string( tokenizer *tok )
const wchar_t *start;
int len;
int mode=0;
wchar_t prev;
int do_loop=1;
int paran_count=0;
@ -383,7 +382,6 @@ static void read_string( tokenizer *tok )
if( !do_loop )
break;
prev = *tok->buff;
tok->buff++;
}

View file

@ -107,7 +107,6 @@ static std::map<wcstring, wcstring> suffix_map;
int wildcard_has( const wchar_t *str, int internal )
{
wchar_t prev=0;
if( !str )
{
debug( 2, L"Got null string on line %d of file %s", __LINE__, __FILE__ );
@ -120,11 +119,11 @@ int wildcard_has( const wchar_t *str, int internal )
{
if( ( *str == ANY_CHAR ) || (*str == ANY_STRING) || (*str == ANY_STRING_RECURSIVE) )
return 1;
prev = *str;
}
}
else
{
wchar_t prev=0;
for( ; *str; str++ )
{
if( ( (*str == L'*' ) || (*str == L'?' ) ) && (prev != L'\\') )