Merge pull request scp-fs2open#6713 from Goober5000/various_small_issues

Various Coverity issues

Author: Goober5000

code/actions/expression/ExpressionParser.cpp

@@ -122,9 +122,9 @@ class ExpressionBuilderVisitor : public ActionExpressionVisitor {
 	}
 	antlrcpp::Any visitVec3d_constructor(ActionExpressionParser::Vec3d_constructorContext* context) override
 	{
-		auto xExpression = visit(context->expression(0)).as<std::shared_ptr<nodes::AbstractExpression>>();
-		auto yExpression = visit(context->expression(1)).as<std::shared_ptr<nodes::AbstractExpression>>();
-		auto zExpression = visit(context->expression(2)).as<std::shared_ptr<nodes::AbstractExpression>>();
+		const auto xExpression = visit(context->expression(0)).as<std::shared_ptr<nodes::AbstractExpression>>();
+		const auto yExpression = visit(context->expression(1)).as<std::shared_ptr<nodes::AbstractExpression>>();
+		const auto zExpression = visit(context->expression(2)).as<std::shared_ptr<nodes::AbstractExpression>>();
 
 		auto expression = std::make_shared<nodes::VectorConstructorExpression>(context->L_PAREN()->getSymbol(),
 			xExpression,

code/actions/types/MoveToSubmodel.cpp

@@ -55,7 +55,7 @@ ActionResult MoveToSubmodel::execute(ProgramLocals& locals) const
 
 	// We need to do a linear search for the right subobject
 	for (int i = 0; i < pm->n_models; ++i) {
-		const auto submodel = pm->submodel[i];
+		const auto& submodel = pm->submodel[i];
 
 		if (subsystem_stricmp(destinationSubObject.c_str(), submodel.name) == 0) {
 			// Found something!

code/controlconfig/controlsconfig.cpp

@@ -1348,7 +1348,7 @@ bool control_config_accept(bool API_Access)
 
 			// Pack the current bindings into a preset, then save the file
 			CC_preset preset;
-			preset.name = str;
+			preset.name = std::move(str);
 			std::copy(Control_config.begin(), Control_config.end(), std::back_inserter(preset.bindings));
 			Control_config_presets.push_back(preset);
 			save_preset_file(preset, true);

code/hud/hudbrackets.cpp

@@ -398,7 +398,6 @@ void HudGaugeBrackets::renderObjectBrackets(object *targetp, color *clr, int w_c
 	int x1,x2,y1,y2;
 	bool draw_box = true;
 	int bound_rc;
-	SCP_list<CJumpNode>::iterator jnp;
 
 
 	bool not_player_target = (Player_ai->target_objnum < 0 || targetp != &Objects[Player_ai->target_objnum]);
@@ -449,13 +448,11 @@ void HudGaugeBrackets::renderObjectBrackets(object *targetp, color *clr, int w_c
 			break;
 
 		case OBJ_JUMP_NODE:
-			for (jnp = Jump_nodes.begin(); jnp != Jump_nodes.end(); ++jnp) {
-				if(jnp->GetSCPObject() == targetp)
-					break;
-			}	
-				
-			modelnum = jnp->GetModelNumber();
+			{
+			auto jnp = jumpnode_get_by_objp(targetp);
+			modelnum = jnp ? jnp->GetModelNumber() : -1;
 			bound_rc = model_find_2d_bound_min( modelnum, &targetp->orient, &targetp->pos,&x1,&y1,&x2,&y2 );
+			}
 			break;
 
 		default:
@@ -637,7 +634,6 @@ void HudGaugeBrackets::renderBoundingBrackets(int x1, int y1, int x2, int y2, in
 		const char* tinfo_class = NULL;
 		char temp_name[NAME_LENGTH*2+3];
 		char temp_class[NAME_LENGTH];
-		SCP_list<CJumpNode>::iterator jnp;
 
 		switch(t_objp->type) {
 			case OBJ_SHIP:
@@ -676,11 +672,8 @@ void HudGaugeBrackets::renderBoundingBrackets(int x1, int y1, int x2, int y2, in
 				}
 				break;
 			case OBJ_JUMP_NODE:
-				for (jnp = Jump_nodes.begin(); jnp != Jump_nodes.end(); ++jnp) {
-					if(jnp->GetSCPObjectNumber() == target_objnum)
-						break;
-				}
-				tinfo_name = jnp->GetDisplayName();
+				auto jnp = jumpnode_get_by_objnum(target_objnum);
+				tinfo_name = jnp ? jnp->GetDisplayName() : "";
 				break;
 		}
 

code/hud/hudsquadmsg.cpp

@@ -207,7 +207,7 @@ void hud_squadmsg_start()
 		auto paramList = scripting::hook_param_list(scripting::hook_param("Player", 'o', Player_obj));
 		if (scripting::hooks::OnHudCommMenuOpened->isOverride(paramList))
 		{
-			scripting::hooks::OnHudCommMenuOpened->run(paramList);
+			scripting::hooks::OnHudCommMenuOpened->run(std::move(paramList));
 			return;
 		}
 	}
@@ -243,7 +243,7 @@ void hud_squadmsg_end()
 		auto paramList = scripting::hook_param_list(scripting::hook_param("Player", 'o', Player_obj));
 		if (scripting::hooks::OnHudCommMenuClosed->isOverride(paramList))
 		{
-			scripting::hooks::OnHudCommMenuClosed->run(paramList);
+			scripting::hooks::OnHudCommMenuClosed->run(std::move(paramList));
 			return;
 		}
 	}
@@ -2007,7 +2007,7 @@ void hud_squadmsg_reinforcement_select()
 			}
 
 			Assert ( Num_menu_items < MAX_MENU_ITEMS );
-			MsgItems[Num_menu_items].text = rp_name;
+			MsgItems[Num_menu_items].text = std::move(rp_name);
 			MsgItems[Num_menu_items].instance = i;
 			MsgItems[Num_menu_items].active = 0;
 

code/hud/hudtarget.cpp

@@ -620,16 +620,11 @@ void hud_reticle_list_update(object *objp, float measure, int dot_flag)
 {
 	reticle_list	*rl, *new_rl;
 	int				i;
-	SCP_list<CJumpNode>::iterator jnp;
 
 	if (objp->type == OBJ_JUMP_NODE) {
-		for (jnp = Jump_nodes.begin(); jnp != Jump_nodes.end(); ++jnp) {
-			if( jnp->GetSCPObject() != objp )
-				continue;
-
-			if( jnp->IsHidden() )
-				return;
-		}
+		auto jnp = jumpnode_get_by_objp(objp);
+		if (!jnp || jnp->IsHidden())
+			return;
 	}
 
 	for ( rl = GET_FIRST(&Reticle_cur_list); rl != END_OF_LIST(&Reticle_cur_list); rl = GET_NEXT(rl) ) {
@@ -1168,7 +1163,6 @@ void hud_target_common(int team_mask, int next_flag)
 	object	*A, *start, *start2;
 	ship		*shipp;
 	int		is_ship, target_found = FALSE;
-	SCP_list<CJumpNode>::iterator jnp;
 
 	if (Player_ai->target_objnum == -1)
 		start = &obj_used_list;
@@ -1209,14 +1203,10 @@ void hud_target_common(int team_mask, int next_flag)
 		}
 
 		if (A->type == OBJ_JUMP_NODE) {
-			for (jnp = Jump_nodes.begin(); jnp != Jump_nodes.end(); ++jnp) {
-				if( jnp->GetSCPObject() == A )
-					break;
-			}
+			auto jnp = jumpnode_get_by_objp(A);
+			Assertion(jnp, "Failed to find jump node with object index %d; trace out and fix!\n", OBJ_INDEX(A));
 
-			Assertion(jnp != Jump_nodes.end(), "Failed to find jump node with object index %d; trace out and fix!\n", OBJ_INDEX(A));
-
-			if( jnp->IsHidden() )
+			if( !jnp || jnp->IsHidden() )
 				continue;
 		}
 
@@ -2419,7 +2409,6 @@ void hud_target_targets_target()
 int object_targetable_in_reticle(object *target_objp)
 {
 	int obj_type;
-	SCP_list<CJumpNode>::iterator jnp;
 
 	if (target_objp == Player_obj ) {
 		return 0;
@@ -2430,12 +2419,12 @@ int object_targetable_in_reticle(object *target_objp)
 	if ( (obj_type == OBJ_SHIP) || (obj_type == OBJ_DEBRIS) || (obj_type == OBJ_WEAPON) || (obj_type == OBJ_ASTEROID) )
 	{
 		return 1;
-	} else if ( obj_type == OBJ_JUMP_NODE )
+	}
+	else if ( obj_type == OBJ_JUMP_NODE )
 	{
-		for (jnp = Jump_nodes.begin(); jnp != Jump_nodes.end(); ++jnp) {
-			if(jnp->GetSCPObject() == target_objp)
-				break;
-		}
+		auto jnp = jumpnode_get_by_objp(target_objp);
+		if (!jnp)
+			return 0;
 
 		if (!jnp->IsHidden())
 			return 1;
@@ -2465,7 +2454,6 @@ void hud_target_in_reticle_new()
 	object	*A;
 	mc_info	mc;
 	float		dist;
-	SCP_list<CJumpNode>::iterator jnp;
 
 	hud_reticle_clear_list(&Reticle_cur_list);
 	Reticle_save_timestamp = timestamp(RESET_TARGET_IN_RETICLE);
@@ -2523,12 +2511,12 @@ void hud_target_in_reticle_new()
 			}
 			break;
 		case OBJ_JUMP_NODE:
-			for (jnp = Jump_nodes.begin(); jnp != Jump_nodes.end(); ++jnp) {
-				if(jnp->GetSCPObject() == A)
-					break;
-			}
-
+			{
+			auto jnp = jumpnode_get_by_objp(A);
+			if (!jnp)
+				continue;
 			mc.model_num = jnp->GetModelNumber();
+			}
 			break;
 		default:
 			Int3();	//	Illegal object type.

code/jumpnode/jumpnode.cpp

@@ -494,6 +494,24 @@ CJumpNode *jumpnode_get_by_objnum(int objnum)
 	return nullptr;
 }
 
+/**
+ * Get jump node object by the object pointer
+ *
+ * @param objp to search for
+ * @return Jump node object pointer
+ */
+CJumpNode *jumpnode_get_by_objp(const object *objp)
+{
+	Assert(objp != nullptr);
+
+	for (CJumpNode &jnp : Jump_nodes) {
+		if (jnp.GetSCPObject() == objp)
+			return &(jnp);
+	}
+
+	return nullptr;
+}
+
 /**
  * Given an object, returns which jump node it's inside (if any)
  *

code/jumpnode/jumpnode.h

@@ -93,6 +93,7 @@ extern SCP_list<CJumpNode> Jump_nodes;
 //-----Functions-----
 CJumpNode *jumpnode_get_by_name(const char *name);
 CJumpNode *jumpnode_get_by_objnum(int objnum);
+CJumpNode *jumpnode_get_by_objp(const object *objp);
 CJumpNode *jumpnode_get_which_in(const object *objp);
 
 void jumpnode_render_all();

code/menuui/credits.cpp

@@ -874,8 +874,8 @@ void credits_do_frame(float  /*frametime*/)
 			// Check if the text part is actually visible
 			if (Credit_position + y_offset + height > 0.0f)
 			{
-				float x = static_cast<float>((gr_screen.clip_width_unscaled - width) / 2);
-				gr_string(x, Credit_position + y_offset, iter->c_str() + currentPos, GR_RESIZE_MENU, 1.0f, length);
+				int x = (gr_screen.clip_width_unscaled - width) / 2;
+				gr_string(i2fl(x), Credit_position + y_offset, iter->c_str() + currentPos, GR_RESIZE_MENU, 1.0f, length);
 			}
 
 			y_offset += height;

code/mission/missionparse.cpp

@@ -6657,9 +6657,9 @@ bool post_process_mission(mission *pm)
 		if (MULTI_TEAM) {
 			team = Net_player->p_info.team;
 		}
-		auto br = Briefings[team].stages;
+		const auto &br = Briefings[team].stages;
 		for (i = 0; i < Briefings[team].num_stages; i++) {
-			auto stage = br[i];
+			const auto &stage = br[i];
 			for (int j = 0; j < stage.num_icons; j++) {
 				stage.icons[j].modelnum = model_load(Ship_info[stage.icons[j].ship_class].pof_file);
 			}