From df5d163f9cddf186e79e271342d7725b981ec196 Mon Sep 17 00:00:00 2001 From: Keith Winstein Date: Thu, 8 Mar 2012 10:50:19 -0500 Subject: [PATCH] Fix asserts with side-effects (per Keegan McAllister) --- src/examples/Makefile.am | 2 +- src/examples/ntester.cc | 3 ++- src/examples/termemu.cc | 5 ++-- src/frontend/mosh-server.cc | 19 ++++++++-------- src/frontend/stmclient.cc | 15 ++++++------ src/network/compressor.cc | 14 ++++++------ src/network/transportfragment.cc | 5 ++-- src/statesync/Makefile.am | 2 +- src/statesync/completeterminal.cc | 3 ++- src/statesync/user.cc | 4 +++- src/terminal/parser.cc | 1 - src/terminal/terminal.cc | 1 + src/terminal/terminaldisplay.cc | 1 - src/util/Makefile.am | 2 +- src/util/fatal_assert.h | 38 +++++++++++++++++++++++++++++++ 15 files changed, 80 insertions(+), 35 deletions(-) create mode 100644 src/util/fatal_assert.h diff --git a/src/examples/Makefile.am b/src/examples/Makefile.am index 46232ae..564a4fd 100644 --- a/src/examples/Makefile.am +++ b/src/examples/Makefile.am @@ -27,5 +27,5 @@ else endif ntester_SOURCES = ntester.cc -ntester_CPPFLAGS = -I$(srcdir)/../statesync -I$(srcdir)/../terminal -I$(srcdir)/../network -I$(srcdir)/../crypto -I../protobufs $(BOOST_CPPFLAGS) +ntester_CPPFLAGS = -I$(srcdir)/../util -I$(srcdir)/../statesync -I$(srcdir)/../terminal -I$(srcdir)/../network -I$(srcdir)/../crypto -I../protobufs $(BOOST_CPPFLAGS) ntester_LDADD = ../statesync/libmoshstatesync.a ../terminal/libmoshterminal.a ../network/libmoshnetwork.a ../crypto/libmoshcrypto.a ../protobufs/libmoshprotos.a -lutil -lm $(BOOST_LDFLAGS) $(protobuf_LIBS) diff --git a/src/examples/ntester.cc b/src/examples/ntester.cc index 08beea4..3f67dd5 100644 --- a/src/examples/ntester.cc +++ b/src/examples/ntester.cc @@ -21,6 +21,7 @@ #include #include "user.h" +#include "fatal_assert.h" #include "networktransport.cc" using namespace Network; @@ -117,7 +118,7 @@ int main( int argc, char *argv[] ) if ( fds[ 0 ].revents & POLLIN ) { char x; - assert( read( STDIN_FILENO, &x, 1 ) == 1 ); + fatal_assert( read( STDIN_FILENO, &x, 1 ) == 1 ); n->get_current_state().push_back( Parser::UserByte( x ) ); } diff --git a/src/examples/termemu.cc b/src/examples/termemu.cc index 0732137..7439bbf 100644 --- a/src/examples/termemu.cc +++ b/src/examples/termemu.cc @@ -49,6 +49,7 @@ #include "parser.h" #include "completeterminal.h" #include "swrite.h" +#include "fatal_assert.h" extern "C" { #include "selfpipe.h" @@ -202,7 +203,7 @@ void emulate_terminal( int fd ) return; } - assert( selfpipe_trap(SIGWINCH) == 0 ); + fatal_assert( selfpipe_trap(SIGWINCH) == 0 ); /* get current window size */ struct winsize window_size; @@ -287,7 +288,7 @@ void emulate_terminal( int fd ) } } else if ( pollfds[ 2 ].revents & POLLIN ) { /* resize */ - assert( selfpipe_read() == SIGWINCH ); + fatal_assert( selfpipe_read() == SIGWINCH ); /* get new size */ if ( ioctl( STDIN_FILENO, TIOCGWINSZ, &window_size ) < 0 ) { diff --git a/src/frontend/mosh-server.cc b/src/frontend/mosh-server.cc index e87f996..b8ac424 100644 --- a/src/frontend/mosh-server.cc +++ b/src/frontend/mosh-server.cc @@ -46,6 +46,7 @@ extern "C" { #include "completeterminal.h" #include "swrite.h" #include "user.h" +#include "fatal_assert.h" #if HAVE_PTY_H #include @@ -109,10 +110,10 @@ int main( int argc, char *argv[] ) /* don't let signals kill us */ sigset_t signals_to_block; - assert( sigemptyset( &signals_to_block ) == 0 ); - assert( sigaddset( &signals_to_block, SIGHUP ) == 0 ); - assert( sigaddset( &signals_to_block, SIGPIPE ) == 0 ); - assert( sigprocmask( SIG_BLOCK, &signals_to_block, NULL ) == 0 ); + fatal_assert( sigemptyset( &signals_to_block ) == 0 ); + fatal_assert( sigaddset( &signals_to_block, SIGHUP ) == 0 ); + fatal_assert( sigaddset( &signals_to_block, SIGPIPE ) == 0 ); + fatal_assert( sigprocmask( SIG_BLOCK, &signals_to_block, NULL ) == 0 ); struct termios child_termios; @@ -153,8 +154,8 @@ int main( int argc, char *argv[] ) /* unblock signals */ sigset_t signals_to_block; - assert( sigemptyset( &signals_to_block ) == 0 ); - assert( sigprocmask( SIG_SETMASK, &signals_to_block, NULL ) == 0 ); + fatal_assert( sigemptyset( &signals_to_block ) == 0 ); + fatal_assert( sigprocmask( SIG_SETMASK, &signals_to_block, NULL ) == 0 ); /* set TERM */ if ( setenv( "TERM", "xterm", true ) < 0 ) { @@ -183,7 +184,7 @@ int main( int argc, char *argv[] ) char *my_argv[ 2 ]; my_argv[ 0 ] = strdup( pw->pw_shell ); - assert( my_argv[ 0 ] ); + fatal_assert( my_argv[ 0 ] ); my_argv[ 1 ] = NULL; if ( execv( pw->pw_shell, my_argv ) < 0 ) { @@ -235,8 +236,8 @@ void serve( int host_fd, Terminal::Complete &terminal, ServerConnection &network return; } - assert( selfpipe_trap( SIGTERM ) == 0 ); - assert( selfpipe_trap( SIGINT ) == 0 ); + fatal_assert( selfpipe_trap( SIGTERM ) == 0 ); + fatal_assert( selfpipe_trap( SIGINT ) == 0 ); /* prepare to poll for events */ struct pollfd pollfds[ 3 ]; diff --git a/src/frontend/stmclient.cc b/src/frontend/stmclient.cc index 8569d0d..66c5729 100644 --- a/src/frontend/stmclient.cc +++ b/src/frontend/stmclient.cc @@ -46,6 +46,7 @@ extern "C" { #include "swrite.h" #include "completeterminal.h" #include "user.h" +#include "fatal_assert.h" #include "networktransport.cc" @@ -111,13 +112,13 @@ void STMClient::main_init( void ) return; } - assert( selfpipe_trap( SIGWINCH ) == 0 ); - assert( selfpipe_trap( SIGTERM ) == 0 ); - assert( selfpipe_trap( SIGINT ) == 0 ); - assert( selfpipe_trap( SIGHUP ) == 0 ); - assert( selfpipe_trap( SIGPIPE ) == 0 ); - assert( selfpipe_trap( SIGTSTP ) == 0 ); - assert( selfpipe_trap( SIGCONT ) == 0 ); + fatal_assert( selfpipe_trap( SIGWINCH ) == 0 ); + fatal_assert( selfpipe_trap( SIGTERM ) == 0 ); + fatal_assert( selfpipe_trap( SIGINT ) == 0 ); + fatal_assert( selfpipe_trap( SIGHUP ) == 0 ); + fatal_assert( selfpipe_trap( SIGPIPE ) == 0 ); + fatal_assert( selfpipe_trap( SIGTSTP ) == 0 ); + fatal_assert( selfpipe_trap( SIGCONT ) == 0 ); /* get initial window size */ if ( ioctl( STDIN_FILENO, TIOCGWINSZ, &window_size ) < 0 ) { diff --git a/src/network/compressor.cc b/src/network/compressor.cc index d40bf03..9f9500f 100644 --- a/src/network/compressor.cc +++ b/src/network/compressor.cc @@ -1,7 +1,7 @@ #include -#include #include "compressor.h" +#include "dos_assert.h" using namespace Network; using namespace std; @@ -9,18 +9,18 @@ using namespace std; string Compressor::compress_str( const string input ) { long unsigned int len = BUFFER_SIZE; - assert( Z_OK == compress( buffer, &len, - reinterpret_cast( input.data() ), - input.size() ) ); + dos_assert( Z_OK == compress( buffer, &len, + reinterpret_cast( input.data() ), + input.size() ) ); return string( reinterpret_cast( buffer ), len ); } string Compressor::uncompress_str( const string input ) { long unsigned int len = BUFFER_SIZE; - assert( Z_OK == uncompress( buffer, &len, - reinterpret_cast( input.data() ), - input.size() ) ); + dos_assert( Z_OK == uncompress( buffer, &len, + reinterpret_cast( input.data() ), + input.size() ) ); return string( reinterpret_cast( buffer ), len ); } diff --git a/src/network/transportfragment.cc b/src/network/transportfragment.cc index 04a6884..297dcb8 100644 --- a/src/network/transportfragment.cc +++ b/src/network/transportfragment.cc @@ -22,6 +22,7 @@ #include "transportfragment.h" #include "transportinstruction.pb.h" #include "compressor.h" +#include "fatal_assert.h" using namespace Network; using namespace TransportBuffers; @@ -46,7 +47,7 @@ string Fragment::tostring( void ) ret += network_order_string( id ); - assert( !( fragment_num & 0x8000 ) ); /* effective limit on size of a terminal screen change or buffered user input */ + fatal_assert( !( fragment_num & 0x8000 ) ); /* effective limit on size of a terminal screen change or buffered user input */ uint16_t combined_fragment_num = ( final << 15 ) | fragment_num; ret += network_order_string( combined_fragment_num ); @@ -122,7 +123,7 @@ Instruction FragmentAssembly::get_assembly( void ) } Instruction ret; - assert( ret.ParseFromString( get_compressor().uncompress_str( encoded ) ) ); + fatal_assert( ret.ParseFromString( get_compressor().uncompress_str( encoded ) ) ); fragments.clear(); fragments_arrived = 0; diff --git a/src/statesync/Makefile.am b/src/statesync/Makefile.am index 445f1f7..760c6f2 100644 --- a/src/statesync/Makefile.am +++ b/src/statesync/Makefile.am @@ -1,4 +1,4 @@ -AM_CPPFLAGS = -I$(srcdir)/../terminal -I../protobufs $(BOOST_CPPFLAGS) +AM_CPPFLAGS = -I$(srcdir)/../util -I$(srcdir)/../terminal -I../protobufs $(BOOST_CPPFLAGS) AM_CXXFLAGS = $(WARNING_CXXFLAGS) $(PICKY_CXXFLAGS) -fno-default-inline -pipe noinst_LIBRARIES = libmoshstatesync.a diff --git a/src/statesync/completeterminal.cc b/src/statesync/completeterminal.cc index f473129..d232519 100644 --- a/src/statesync/completeterminal.cc +++ b/src/statesync/completeterminal.cc @@ -20,6 +20,7 @@ #include #include "completeterminal.h" +#include "fatal_assert.h" #include "hostinput.pb.h" @@ -83,7 +84,7 @@ string Complete::diff_from( const Complete &existing ) const void Complete::apply_string( string diff ) { HostBuffers::HostMessage input; - assert( input.ParseFromString( diff ) ); + fatal_assert( input.ParseFromString( diff ) ); for ( int i = 0; i < input.instruction_size(); i++ ) { if ( input.instruction( i ).HasExtension( hostbytes ) ) { diff --git a/src/statesync/user.cc b/src/statesync/user.cc index d32aca3..90ebe5a 100644 --- a/src/statesync/user.cc +++ b/src/statesync/user.cc @@ -20,6 +20,7 @@ #include #include "user.h" +#include "fatal_assert.h" #include "userinput.pb.h" using namespace Parser; @@ -75,6 +76,7 @@ string UserStream::diff_from( const UserStream &existing ) const break; default: assert( false ); + break; } my_it++; @@ -86,7 +88,7 @@ string UserStream::diff_from( const UserStream &existing ) const void UserStream::apply_string( string diff ) { ClientBuffers::UserMessage input; - assert( input.ParseFromString( diff ) ); + fatal_assert( input.ParseFromString( diff ) ); for ( int i = 0; i < input.instruction_size(); i++ ) { if ( input.instruction( i ).HasExtension( keystroke ) ) { diff --git a/src/terminal/parser.cc b/src/terminal/parser.cc index f344a74..d2c5594 100644 --- a/src/terminal/parser.cc +++ b/src/terminal/parser.cc @@ -68,7 +68,6 @@ std::list Parser::UTF8Parser::input( char c ) buf[ buf_len++ ] = c; /* This function will only work in a UTF-8 locale. */ - /* This is asserted in the constructor. */ wchar_t pwc; mbstate_t ps; diff --git a/src/terminal/terminal.cc b/src/terminal/terminal.cc index 6345b1b..15a5c3a 100644 --- a/src/terminal/terminal.cc +++ b/src/terminal/terminal.cc @@ -123,6 +123,7 @@ void Emulator::print( const Parser::Print *act ) break; default: assert( false ); + break; } } diff --git a/src/terminal/terminaldisplay.cc b/src/terminal/terminaldisplay.cc index 9fc6cac..cce71a4 100644 --- a/src/terminal/terminaldisplay.cc +++ b/src/terminal/terminaldisplay.cc @@ -17,7 +17,6 @@ */ #include -#include #include #include "terminaldisplay.h" diff --git a/src/util/Makefile.am b/src/util/Makefile.am index 815f87d..828b181 100644 --- a/src/util/Makefile.am +++ b/src/util/Makefile.am @@ -2,4 +2,4 @@ AM_CXXFLAGS = $(WARNING_CXXFLAGS) $(PICKY_CXXFLAGS) -fno-default-inline -pipe noinst_LIBRARIES = libmoshutil.a -libmoshutil_a_SOURCES = swrite.cc swrite.h dos_assert.h +libmoshutil_a_SOURCES = swrite.cc swrite.h dos_assert.h fatal_assert.h diff --git a/src/util/fatal_assert.h b/src/util/fatal_assert.h new file mode 100644 index 0000000..dc41f64 --- /dev/null +++ b/src/util/fatal_assert.h @@ -0,0 +1,38 @@ +/* + Mosh: the mobile shell + Copyright 2012 Keith Winstein + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#ifndef FATAL_ASSERT_HPP +#define FATAL_ASSERT_HPP + +#include +#include + +static void fatal_error( const char *expression, const char *file, int line, const char *function ) +{ + char buffer[ 2048 ]; + snprintf( buffer, 2048, "Fatal assertion failure in function %s at %s:%d, failed test: %s\n", + function, file, line, expression ); + exit( 2 ); +} + +#define fatal_assert(expr) \ + ((expr) \ + ? (void)0 \ + : fatal_error (__STRING(expr), __FILE__, __LINE__, __PRETTY_FUNCTION__ )) + +#endif