Prefer early exit/break to large conditional blocks.

Some of these are large and contain smaller unrelated refactors.

src/frontend/stmclient.cc

@@ -314,82 +314,82 @@ bool STMClient::process_user_input( int fd )
 
   NetworkType &net = *network;
 
-  if ( !net.shutdown_in_progress() ) {
+  if ( net.shutdown_in_progress() ) {
-    overlays.get_prediction_engine().set_local_frame_sent( net.get_sent_state_last() );
+    return true;
+  }
+  overlays.get_prediction_engine().set_local_frame_sent( net.get_sent_state_last() );
 
-    /* Don't predict for bulk data. */
+  /* Don't predict for bulk data. */
-    bool paste = bytes_read > 100;
+  bool paste = bytes_read > 100;
-    if ( paste ) {
+  if ( paste ) {
-      overlays.get_prediction_engine().reset();
+    overlays.get_prediction_engine().reset();
+  }
+
+  for ( int i = 0; i < bytes_read; i++ ) {
+    char the_byte = buf[ i ];
+
+    if ( !paste ) {
+      overlays.get_prediction_engine().new_user_byte( the_byte, local_framebuffer );
     }
 
-    for ( int i = 0; i < bytes_read; i++ ) {
+    if ( quit_sequence_started ) {
-      char the_byte = buf[ i ];
+      if ( the_byte == '.' ) { /* Quit sequence is Ctrl-^ . */
+	if ( net.has_remote_addr() && (!net.shutdown_in_progress()) ) {
+	  overlays.get_notification_engine().set_notification_string( wstring( L"Exiting on user request..." ), true );
+	  net.start_shutdown();
+	  return true;
+	}
+	return false;
+      } else if ( the_byte == 0x1a ) { /* Suspend sequence is escape_key Ctrl-Z */
+	/* Restore terminal and terminal-driver state */
+	swrite( STDOUT_FILENO, display.close().c_str() );
 
-      if ( !paste ) {
+	if ( tcsetattr( STDIN_FILENO, TCSANOW, &saved_termios ) < 0 ) {
-	overlays.get_prediction_engine().new_user_byte( the_byte, local_framebuffer );
+	  perror( "tcsetattr" );
-      }
+	  exit( 1 );
-
-      if ( quit_sequence_started ) {
-	if ( the_byte == '.' ) { /* Quit sequence is Ctrl-^ . */
-	  if ( net.has_remote_addr() && (!net.shutdown_in_progress()) ) {
-	    overlays.get_notification_engine().set_notification_string( wstring( L"Exiting on user request..." ), true );
-	    net.start_shutdown();
-	    return true;
-	  } else {
-	    return false;
-	  }
-	} else if ( the_byte == 0x1a ) { /* Suspend sequence is escape_key Ctrl-Z */
-	  /* Restore terminal and terminal-driver state */
-	  swrite( STDOUT_FILENO, display.close().c_str() );
-
-	  if ( tcsetattr( STDIN_FILENO, TCSANOW, &saved_termios ) < 0 ) {
-	    perror( "tcsetattr" );
-	    exit( 1 );
-	  }
-
-	  fputs( "\n\033[37;44m[mosh is suspended.]\033[m\n", stdout );
-
-	  fflush( NULL );
-
-	  /* actually suspend */
-	  kill( 0, SIGSTOP );
-
-	  resume();
-	} else if ( (the_byte == escape_pass_key) || (the_byte == escape_pass_key2) ) {
-	  /* Emulation sequence to type escape_key is escape_key +
-	     escape_pass_key (that is escape key without Ctrl) */
-	  net.get_current_state().push_back( Parser::UserByte( escape_key ) );
-	} else {
-	  /* Escape key followed by anything other than . and ^ gets sent literally */
-	  net.get_current_state().push_back( Parser::UserByte( escape_key ) );
-	  net.get_current_state().push_back( Parser::UserByte( the_byte ) );
 	}
 
-	quit_sequence_started = false;
+	fputs( "\n\033[37;44m[mosh is suspended.]\033[m\n", stdout );
 
-	if ( overlays.get_notification_engine().get_notification_string() == escape_key_help ) {
+	fflush( NULL );
-	  overlays.get_notification_engine().set_notification_string( L"" );
-	}
 
-	continue;
+	/* actually suspend */
+	kill( 0, SIGSTOP );
+
+	resume();
+      } else if ( (the_byte == escape_pass_key) || (the_byte == escape_pass_key2) ) {
+	/* Emulation sequence to type escape_key is escape_key +
+	   escape_pass_key (that is escape key without Ctrl) */
+	net.get_current_state().push_back( Parser::UserByte( escape_key ) );
+      } else {
+	/* Escape key followed by anything other than . and ^ gets sent literally */
+	net.get_current_state().push_back( Parser::UserByte( escape_key ) );
+	net.get_current_state().push_back( Parser::UserByte( the_byte ) );	  
       }
 
-      quit_sequence_started = (escape_key > 0) && (the_byte == escape_key) && (lf_entered || (! escape_requires_lf));
+      quit_sequence_started = false;
-      if ( quit_sequence_started ) {
+
-	lf_entered = false;
+      if ( overlays.get_notification_engine().get_notification_string() == escape_key_help ) {
-	overlays.get_notification_engine().set_notification_string( escape_key_help, true, false );
+	overlays.get_notification_engine().set_notification_string( L"" );
-	continue;
       }
 
-      lf_entered = ( (the_byte == 0x0A) || (the_byte == 0x0D) ); /* LineFeed, Ctrl-J, '\n' or CarriageReturn, Ctrl-M, '\r' */
+      continue;
-
-      if ( the_byte == 0x0C ) { /* Ctrl-L */
-	repaint_requested = true;
-      }
-
-      net.get_current_state().push_back( Parser::UserByte( the_byte ) );
     }
+
+    quit_sequence_started = (escape_key > 0) && (the_byte == escape_key) && (lf_entered || (! escape_requires_lf));
+    if ( quit_sequence_started ) {
+      lf_entered = false;
+      overlays.get_notification_engine().set_notification_string( escape_key_help, true, false );
+      continue;
+    }
+
+    lf_entered = ( (the_byte == 0x0A) || (the_byte == 0x0D) ); /* LineFeed, Ctrl-J, '\n' or CarriageReturn, Ctrl-M, '\r' */
+
+    if ( the_byte == 0x0C ) { /* Ctrl-L */
+      repaint_requested = true;
+    }
+
+    net.get_current_state().push_back( Parser::UserByte( the_byte ) );		
   }
 
   return true;

src/frontend/terminaloverlay.cc

@@ -90,32 +90,30 @@ Validity ConditionalOverlayCell::get_validity( const Framebuffer &fb, int row,
   const Cell &current = *( fb.get_cell( row, col ) );
 
   /* see if it hasn't been updated yet */
-  if ( late_ack >= expiration_frame ) {
+  if ( late_ack < expiration_frame ) {
-    if ( unknown ) {
+    return Pending;
-      return CorrectNoCredit;
-    }
-
-    if ( replacement.is_blank() ) { /* too easy for this to trigger falsely */
-      return CorrectNoCredit;
-    }
-
-    if ( current.contents_match( replacement ) ) {
-      vector<Cell>::const_iterator it = original_contents.begin();
-      for ( ; it != original_contents.end(); it++ ) {
-        if ( it->contents_match( replacement ) )
-          break;
-      }
-      if ( it == original_contents.end() ) {
-	return Correct;
-      } else {
-	return CorrectNoCredit;
-      }
-    } else {
-      return IncorrectOrExpired;
-    }
   }
 
-  return Pending;
+  if ( unknown ) {
+    return CorrectNoCredit;
+  }
+
+  if ( replacement.is_blank() ) { /* too easy for this to trigger falsely */
+    return CorrectNoCredit;
+  }
+
+  if ( current.contents_match( replacement ) ) {
+    vector<Cell>::const_iterator it = original_contents.begin();
+    for ( ; it != original_contents.end(); it++ ) {
+      if ( it->contents_match( replacement ) )
+	break;
+    }
+    if ( it == original_contents.end() ) {
+      return Correct;
+    }
+    return CorrectNoCredit;
+  }
+  return IncorrectOrExpired;
 }
 
 Validity ConditionalCursorMove::get_validity( const Framebuffer &fb,

src/network/network.cc

@@ -523,66 +523,65 @@ string Connection::recv_one( int sock_to_recv, bool nonblocking )
 
   dos_assert( p.direction == (server ? TO_SERVER : TO_CLIENT) ); /* prevent malicious playback to sender */
 
-  if ( p.seq >= expected_receiver_seq ) { /* don't use out-of-order packets for timestamp or targeting */
+  if ( p.seq < expected_receiver_seq ) { /* don't use (but do return) out-of-order packets for timestamp or targeting */
-    expected_receiver_seq = p.seq + 1; /* this is security-sensitive because a replay attack could otherwise
+    return p.payload;
-					  screw up the timestamp and targeting */
+  }
+  expected_receiver_seq = p.seq + 1; /* this is security-sensitive because a replay attack could otherwise
+					screw up the timestamp and targeting */
 
-    if ( p.timestamp != uint16_t(-1) ) {
+  if ( p.timestamp != uint16_t(-1) ) {
-      saved_timestamp = p.timestamp;
+    saved_timestamp = p.timestamp;
-      saved_timestamp_received_at = timestamp();
+    saved_timestamp_received_at = timestamp();
 
-      if ( congestion_experienced ) {
+    if ( congestion_experienced ) {
-	/* signal counterparty to slow down */
+      /* signal counterparty to slow down */
-	/* this will gradually slow the counterparty down to the minimum frame rate */
+      /* this will gradually slow the counterparty down to the minimum frame rate */
-	saved_timestamp -= CONGESTION_TIMESTAMP_PENALTY;
+      saved_timestamp -= CONGESTION_TIMESTAMP_PENALTY;
-	if ( server ) {
+      if ( server ) {
-	  fprintf( stderr, "Received explicit congestion notification.\n" );
+	fprintf( stderr, "Received explicit congestion notification.\n" );
-	}
-      }
-    }
-
-    if ( p.timestamp_reply != uint16_t(-1) ) {
-      uint16_t now = timestamp16();
-      double R = timestamp_diff( now, p.timestamp_reply );
-
-      if ( R < 5000 ) { /* ignore large values, e.g. server was Ctrl-Zed */
-	if ( !RTT_hit ) { /* first measurement */
-	  SRTT = R;
-	  RTTVAR = R / 2;
-	  RTT_hit = true;
-	} else {
-	  const double alpha = 1.0 / 8.0;
-	  const double beta = 1.0 / 4.0;
-	  
-	  RTTVAR = (1 - beta) * RTTVAR + ( beta * fabs( SRTT - R ) );
-	  SRTT = (1 - alpha) * SRTT + ( alpha * R );
-	}
-      }
-    }
-
-    /* auto-adjust to remote host */
-    has_remote_addr = true;
-    last_heard = timestamp();
-
-    if ( server ) { /* only client can roam */
-      if ( remote_addr_len != header.msg_namelen ||
-	   memcmp( &remote_addr, &packet_remote_addr, remote_addr_len ) != 0 ) {
-	remote_addr = packet_remote_addr;
-	remote_addr_len = header.msg_namelen;
-	char host[ NI_MAXHOST ], serv[ NI_MAXSERV ];
-	int errcode = getnameinfo( &remote_addr.sa, remote_addr_len,
-				   host, sizeof( host ), serv, sizeof( serv ),
-				   NI_DGRAM | NI_NUMERICHOST | NI_NUMERICSERV );
-	if ( errcode != 0 ) {
-	  throw NetworkException( std::string( "recv_one: getnameinfo: " ) + gai_strerror( errcode ), 0 );
-	}
-	fprintf( stderr, "Server now attached to client at %s:%s\n",
-		 host, serv );
       }
     }
   }
 
-  return p.payload; /* we do return out-of-order or duplicated packets to caller */
+  if ( p.timestamp_reply != uint16_t(-1) ) {
+    uint16_t now = timestamp16();
+    double R = timestamp_diff( now, p.timestamp_reply );
+
+    if ( R < 5000 ) { /* ignore large values, e.g. server was Ctrl-Zed */
+      if ( !RTT_hit ) { /* first measurement */
+	SRTT = R;
+	RTTVAR = R / 2;
+	RTT_hit = true;
+      } else {
+	const double alpha = 1.0 / 8.0;
+	const double beta = 1.0 / 4.0;
+	  
+	RTTVAR = (1 - beta) * RTTVAR + ( beta * fabs( SRTT - R ) );
+	SRTT = (1 - alpha) * SRTT + ( alpha * R );
+      }
+    }
+  }
+
+  /* auto-adjust to remote host */
+  has_remote_addr = true;
+  last_heard = timestamp();
+
+  if ( server && /* only client can roam */
+       ( remote_addr_len != header.msg_namelen ||
+	 memcmp( &remote_addr, &packet_remote_addr, remote_addr_len ) != 0 ) ) {
+    remote_addr = packet_remote_addr;
+    remote_addr_len = header.msg_namelen;
+    char host[ NI_MAXHOST ], serv[ NI_MAXSERV ];
+    int errcode = getnameinfo( &remote_addr.sa, remote_addr_len,
+			       host, sizeof( host ), serv, sizeof( serv ),
+			       NI_DGRAM | NI_NUMERICHOST | NI_NUMERICSERV );
+    if ( errcode != 0 ) {
+      throw NetworkException( std::string( "recv_one: getnameinfo: " ) + gai_strerror( errcode ), 0 );
+    }
+    fprintf( stderr, "Server now attached to client at %s:%s\n",
+	     host, serv );
+  }
+  return p.payload;
 }
 
 std::string Connection::port( void ) const

src/terminal/terminaldisplay.cc

@@ -181,29 +181,30 @@ std::string Display::new_frame( bool initialized, const Framebuffer &last, const
     for ( int row = 0; row < f.ds.get_height(); row++ ) {
       const Row *new_row = f.get_row( 0 );
       const Row *old_row = &*rows.at( row );
-      if ( new_row == old_row || *new_row == *old_row ) {
+      if ( ! ( new_row == old_row || *new_row == *old_row ) ) {
-	/* if row 0, we're looking at ourselves and probably didn't scroll */
+	continue;
-	if ( row == 0 ) {
+      }
-	  break;
+      /* if row 0, we're looking at ourselves and probably didn't scroll */
-	}
+      if ( row == 0 ) {
-	/* found a scroll */
-	lines_scrolled = row;
-	scroll_height = 1;
-
-	/* how big is the region that was scrolled? */
-	for ( int region_height = 1;
-	      lines_scrolled + region_height < f.ds.get_height();
-	      region_height++ ) {
-	  if ( *f.get_row( region_height )
-	       == *rows.at( lines_scrolled + region_height ) ) {
-	    scroll_height = region_height + 1;
-	  } else {
-	    break;
-	  }
-	}
-
 	break;
       }
+      /* found a scroll */
+      lines_scrolled = row;
+      scroll_height = 1;
+
+      /* how big is the region that was scrolled? */
+      for ( int region_height = 1;
+	    lines_scrolled + region_height < f.ds.get_height();
+	    region_height++ ) {
+	if ( *f.get_row( region_height )
+	     == *rows.at( lines_scrolled + region_height ) ) {
+	  scroll_height = region_height + 1;
+	} else {
+	  break;
+	}
+      }
+
+      break;
     }
 
     if ( scroll_height ) {
@@ -458,23 +459,23 @@ bool Display::put_row( bool initialized, FrameState &frame, const Framebuffer &f
     }
   }
 
-  if ( wrote_last_cell
+  if ( ! ( wrote_last_cell
-       && (frame_y < f.ds.get_height() - 1) ) {
+	   && (frame_y < f.ds.get_height() - 1) ) ) {
-    /* To hint that a word-select should group the end of one line
+    return false;
-       with the beginning of the next, we let the real cursor
-       actually wrap around in cases where it wrapped around for us. */
-    if ( wrap_this ) {
-      /* Update our cursor, and ask for wrap on the next row. */
-      frame.cursor_x = 0;
-      frame.cursor_y++;
-      return true;
-    } else {
-      /* Resort to CR/LF and update our cursor. */
-      frame.append( "\r\n" );
-      frame.cursor_x = 0;
-      frame.cursor_y++;
-    }
   }
+  /* To hint that a word-select should group the end of one line
+     with the beginning of the next, we let the real cursor
+     actually wrap around in cases where it wrapped around for us. */
+  if ( wrap_this ) {
+    /* Update our cursor, and ask for wrap on the next row. */
+    frame.cursor_x = 0;
+    frame.cursor_y++;
+    return true;
+  }
+  /* Resort to CR/LF and update our cursor. */
+  frame.append( "\r\n" );
+  frame.cursor_x = 0;
+  frame.cursor_y++;
   return false;
 }