Several coverity fixes (#6232)

* coverity - using `std::move` instead of copying, and change some parameters from value to reference

* coverity - fix or mark potential copy-paste mistakes

* coverity - various checking of parameter and return values

* coverity - miscellaneous fixes, and change some 'intentional' markers to 'false positive' markers
This commit is contained in:
Goober5000 authored and GitHub committed 2024-07-06 12:51:31 +02:00
1 parent f20c1bddfd
commit e90416e199
26 files changed
+86 -66

No files matched your search

+1 -1
View File
@@ -766,7 +766,7 @@ void asteroid_create_asteroid_field(int num_asteroids, int field_type, int aster
Asteroid_field.inner_max_bound = i_max;
}
Asteroid_field.target_names = targets;
Asteroid_field.target_names = std::move(targets);
// Only create asteroids if we have some to create
if ((!Asteroid_field.field_asteroid_type.empty()) && (num_asteroids > 0)) {
+1 -1
View File
@@ -595,7 +595,7 @@ static void cf_add_mod_roots(const char* rootDirectory, uint32_t basic_location)
cf_root* root = cf_create_root();
root->path = rootPath;
root->path = std::move(rootPath);
if (primary) {
root->location_flags = basic_location | CF_LOCATION_TYPE_PRIMARY_MOD;
} else {
@@ -2784,10 +2784,12 @@ bool CCB::operator!=(const CCB& A) const {
}
bool CCB::has_first_conflict(const CCB& A) const {
// coverity[copy_paste_error:FALSE]
return !first.empty() && (first.conflicts_with(A.first) || first.conflicts_with(A.second));
}
bool CCB::has_second_conflict(const CCB& A) const {
// coverity[copy_paste_error:FALSE]
return !second.empty() && (second.conflicts_with(A.first) || second.conflicts_with(A.second));
}
+1 -1
View File
@@ -122,7 +122,7 @@ void SubtitleDecoder::pushSubtitleFrame(AVPacket* packet, AVSubtitle* subtitle)
}
SubtitleFramePtr frame(new SubtitleFrame());
frame->text = processed_text;
frame->text = std::move(processed_text);
frame->displayStartTime = start_time;
frame->displayEndTime = end_time;
+1 -1
View File
@@ -174,7 +174,7 @@ void parse_decals_table(const char* filename) {
SCP_string name;
stuff_string(name, F_NAME);
DecalDefinition def(name);
DecalDefinition def(std::move(name));
def.parse();
DecalDefinitions.push_back(std::move(def));
+3 -3
View File
@@ -1005,7 +1005,7 @@ void gamesnd_parse_entry(game_snd *gs, bool &orig_no_create, SCP_vector<game_snd
}
}
gs->name = name;
gs->name = std::move(name);
}
else
{
@@ -1013,7 +1013,7 @@ void gamesnd_parse_entry(game_snd *gs, bool &orig_no_create, SCP_vector<game_snd
{
Warning(LOCATION, "No existing sound entry with name \"%s\" found!", name.c_str());
no_create = false;
gs->name = name;
gs->name = std::move(name);
}
else
{
@@ -1299,7 +1299,7 @@ void parse_sound_table(const char* filename)
}
}
Snds.push_back(game_snd(tempSound));
Snds.emplace_back(std::move(tempSound));
}
}
+1 -1
View File
@@ -27,7 +27,7 @@ void Undo_system::clamp_stacks() {
// This while() shouldn't ever be triggered, but this is a safety to prevent any weird edge-cases.
while (redo_stack.size() > max_undos) {
delete redo_stack.front();
undo_stack.pop_front();
redo_stack.pop_front();
}
}
+1 -1
View File
@@ -5520,7 +5520,7 @@ void load_gauge_scripting(gauge_settings* settings) {
SCP_string name;
stuff_string(name, F_NAME);
hud_gauge->initName(name);
hud_gauge->initName(std::move(name));
gauge_assign_common(settings, std::move(hud_gauge));
}
+1 -1
View File
@@ -31,7 +31,7 @@ void LabUi::object_changed()
trigger = false;
}
void LabUi::build_species_entry(species_info species_def, int species_idx) const
void LabUi::build_species_entry(const species_info &species_def, int species_idx) const
{
with_TreeNode(species_def.species_name)
{
+1 -1
View File
@@ -13,7 +13,7 @@ class LabUi {
private:
void build_options_menu();
void build_ship_list() const;
void build_species_entry(species_info species_def, int species_idx) const;
void build_species_entry(const species_info &species_def, int species_idx) const;
void build_weapon_list() const;
void build_weapon_subtype_list() const;
void build_background_list() const;
+1 -1
View File
@@ -28,7 +28,7 @@ void parse_curve_table(const char* filename) {
int index = curve_get_by_name(name);
if (index < 0) {
Curve curv = Curve(name);
Curve curv = Curve(std::move(name));
curv.ParseData();
Curves.push_back(curv);
} else {
+2
View File
@@ -8386,6 +8386,8 @@ int get_special_anchor(const char *name)
strcpy_s(tmp, name+5);
iff_name = strtok(tmp, " >");
if (iff_name == nullptr)
return -1;
// hack substitute "hostile" for "enemy"
if (!stricmp(iff_name, "enemy"))
+1 -1
View File
@@ -340,7 +340,7 @@ void HudGaugeDirectives::render(float /*frametime*/)
if ( second_line ) {
if (directives_top.first_frame >= 0)
if (directives_middle.first_frame >= 0)
renderBitmap(directives_middle.first_frame, bx, by);
by += text_h;
+1 -1
View File
@@ -1377,7 +1377,7 @@ void stuff_string(SCP_string &outstr, int type, const char *terminators)
}
else
{
outstr = read_str;
outstr = std::move(read_str);
}
diag_printf("Stuffed string = [%.30s]\n", outstr.c_str());
+34 -23
View File
@@ -2784,7 +2784,7 @@ int check_sexp_syntax(int node, int return_type, int recursive, int *bad_node, s
SCP_string triggeredBy = CTEXT(CDDR(ship_node));
SCP_tolower(triggeredBy);
if(std::find_if(animations.cbegin(), animations.cend(), [triggerType, triggeredBy](const std::remove_reference<decltype(animations)>::type::value_type& animation) -> bool {
if(std::find_if(animations.cbegin(), animations.cend(), [&triggerType, &triggeredBy](const std::remove_reference<decltype(animations)>::type::value_type& animation) -> bool {
if (animation.type != triggerType)
return false;
@@ -4289,6 +4289,9 @@ void get_sexp_text_for_variable(char *text, int sexp_var_index)
// Goober5000
void do_preload_for_arguments(void (*preloader)(const char *), int arg_node, int arg_handler_node)
{
if (arg_node < 0)
return;
// we have a special argument
if (Sexp_nodes[arg_node].flags & SNF_SPECIAL_ARG_IN_NODE)
{
@@ -5503,7 +5506,8 @@ player *get_player_from_ship_node(int node, bool test_respawns = false, int *net
*/
const ship_registry_entry *eval_ship(int node)
{
Assert(node >= 0);
if (node < 0)
return nullptr;
// check cache
if (Sexp_nodes[node].cache)
@@ -5544,7 +5548,8 @@ const ship_registry_entry *eval_ship(int node)
*/
wing *eval_wing(int node)
{
Assert(node >= 0);
if (node < 0)
return nullptr;
// check cache
if (Sexp_nodes[node].cache)
@@ -5587,7 +5592,8 @@ wing *eval_wing(int node)
int sexp_atoi(int node)
{
Assertion(!Fred_running, "This function relies on SEXP caching which is not set up to work in FRED!");
Assert(node >= 0);
if (node < 0)
return 0;
// check cache
if (Sexp_nodes[node].cache)
@@ -5625,7 +5631,8 @@ int sexp_atoi(int node)
bool sexp_can_construe_as_integer(int node)
{
Assertion(!Fred_running, "This function relies on SEXP caching which is not set up to work in FRED!");
Assert(node >= 0);
if (node < 0)
return false;
if (Sexp_nodes[node].cache && Sexp_nodes[node].cache->sexp_node_data_type == OPF_NUMBER)
return true;
@@ -5650,7 +5657,8 @@ bool sexp_can_construe_as_integer(int node)
*/
int sexp_get_variable_index(int node)
{
Assert(node >= 0);
if (node < 0)
return -1;
if (Sexp_nodes[node].cached_variable_index >= 0)
return Sexp_nodes[node].cached_variable_index;
@@ -6612,11 +6620,10 @@ object_ship_wing_point_team::object_ship_wing_point_team(ship* sp)
ship_registry_index = ship_registry_get_index(sp->ship_name);
Assertion(ship_registry_index >= 0, "Ship %s was not found in the ship registry!", sp->ship_name);
if (Ship_registry[ship_registry_index].status == ShipStatus::EXITED)
{
if (ship_registry_index < 0)
type = OSWPT_TYPE_NONE;
else if (Ship_registry[ship_registry_index].status == ShipStatus::EXITED)
type = OSWPT_TYPE_EXITED;
}
}
object_ship_wing_point_team::object_ship_wing_point_team(p_object* pop)
@@ -6626,6 +6633,8 @@ object_ship_wing_point_team::object_ship_wing_point_team(p_object* pop)
ship_registry_index = ship_registry_get_index(pop->name);
Assertion(ship_registry_index >= 0, "Parse object %s was not found in the ship registry!", pop->name);
if (ship_registry_index < 0)
type = OSWPT_TYPE_NONE;
}
object_ship_wing_point_team::object_ship_wing_point_team(ship_obj* sop)
@@ -6807,7 +6816,7 @@ void eval_object_ship_wing_point_team(object_ship_wing_point_team *oswpt, int no
{
int ship_registry_index = -1;
int wingnum = -1;
const char *object_name = nullptr;
const char *node_ctext = nullptr;
Assert(oswpt != nullptr);
Assert(node >= 0 || ctext_override);
@@ -6822,7 +6831,7 @@ void eval_object_ship_wing_point_team(object_ship_wing_point_team *oswpt, int no
{
wingnum = wing_lookup(ctext_override);
if (wingnum < 0)
object_name = ctext_override;
node_ctext = ctext_override;
}
}
// check caching
@@ -6855,7 +6864,7 @@ void eval_object_ship_wing_point_team(object_ship_wing_point_team *oswpt, int no
if (wingp)
wingnum = WING_INDEX(wingp);
else
object_name = CTEXT(node);
node_ctext = CTEXT(node);
}
}
@@ -6930,7 +6939,8 @@ void eval_object_ship_wing_point_team(object_ship_wing_point_team *oswpt, int no
// check if we have nothing
if(!stricmp(object_name, SEXP_NONE_STRING))
// (node_ctext will be non-NULL if there is no ship or wing - but that's hard to determine, Coverity misses it, and it could easily change in the future if this function is modified)
if (!node_ctext || !stricmp(node_ctext, SEXP_NONE_STRING))
{
oswpt->type = OSWPT_TYPE_NONE;
return;
@@ -6938,7 +6948,7 @@ void eval_object_ship_wing_point_team(object_ship_wing_point_team *oswpt, int no
// check if we have a point for a target
auto wpt = find_matching_waypoint(object_name);
auto wpt = find_matching_waypoint(node_ctext);
if ((wpt != nullptr) && (wpt->get_objnum() >= 0))
{
oswpt->type = OSWPT_TYPE_WAYPOINT;
@@ -6952,7 +6962,7 @@ void eval_object_ship_wing_point_team(object_ship_wing_point_team *oswpt, int no
// check if we have an "<any team>" type
int team = sexp_determine_team(object_name);
int team = sexp_determine_team(node_ctext);
if (team >= 0)
{
oswpt->type = OSWPT_TYPE_SHIP_ON_TEAM;
@@ -6962,7 +6972,7 @@ void eval_object_ship_wing_point_team(object_ship_wing_point_team *oswpt, int no
// check if we have a whole-team type
team = iff_lookup(object_name);
team = iff_lookup(node_ctext);
if (team >= 0)
{
oswpt->type = OSWPT_TYPE_WHOLE_TEAM;
@@ -8118,7 +8128,7 @@ int sexp_is_ship_visible(int node)
if (n >= 0)
{
auto viewer_entry = eval_ship(n);
return ship_check_visibility(ship_entry->shipp(), viewer_entry->shipp());
return ship_check_visibility(ship_entry->shipp(), viewer_entry ? viewer_entry->shipp_or_null() : nullptr);
}
else
{
@@ -9075,6 +9085,7 @@ int sexp_angle_vectors(int node) {
if (is_nan1 || is_nan2)
return SEXP_NAN;
// coverity[uninit_use_in_call:FALSE] - v1 and v2 are always populated by eval_vec3d
if (IS_VEC_NULL(&v1) || IS_VEC_NULL(&v2))
return SEXP_NAN;
@@ -13358,7 +13369,7 @@ void sexp_set_player_orders(int n)
std::set<size_t> diff;
std::set_difference(shipp->orders_accepted.begin(), shipp->orders_accepted.end(), orders.begin(), orders.end(),
std::inserter(diff, diff.end()));
shipp->orders_accepted = diff;
shipp->orders_accepted = std::move(diff);
}
}
@@ -13401,7 +13412,7 @@ void sexp_set_order_allowed_for_target(int n)
std::set<size_t> diff;
std::set_difference(shipp->orders_allowed_against.begin(), shipp->orders_allowed_against.end(), orders.begin(), orders.end(),
std::inserter(diff, diff.end()));
shipp->orders_allowed_against = diff;
shipp->orders_allowed_against = std::move(diff);
}
}
@@ -16397,7 +16408,7 @@ void sexp_set_asteroid_field(int n)
inner_box,
i_min,
i_max,
targets);
std::move(targets));
}
@@ -16491,7 +16502,7 @@ void sexp_set_debris_field(int n)
asteroid_create_debris_field(
num_asteroids,
asteroid_speed,
debris_types,
std::move(debris_types),
o_min,
o_max,
enhanced);
@@ -19026,7 +19037,7 @@ void sexp_weapon_create(int n)
is_locked = (target_objnum >= 0) ? 1 : 0; // assume full lock; this lets lasers track if people want them to
// create the weapon
// coverity[uninit_use_in_call] - weapon_pos is always populated by eval_vec3d
// coverity[uninit_use_in_call:FALSE] - weapon_pos is always populated by eval_vec3d
weapon_objnum = weapon_create(&weapon_pos, &weapon_orient, weapon_class, parent_objnum, -1, is_locked);
// maybe make the weapon track its target
+1 -1
View File
@@ -54,7 +54,7 @@ void parse_sexp_table(const char* filename) {
dynamic_sexp_enum_list thisList;
thisList.name = name;
thisList.name = std::move(name);
while (optional_string("+Enum:")) {
SCP_string item;
+1 -1
View File
@@ -264,7 +264,7 @@ ParticleSourceWrapper ParticleManager::createSource(ParticleEffectHandle index)
// To ensure this we reserve the number of sources we will need (current sources + sources being created)
if (m_processingSources) {
// If we are already in our onFrame, we need to apply the hack to the right vector though
m_deferredSourceAdding.reserve(m_sources.size() + childEffects.size());
m_deferredSourceAdding.reserve(m_deferredSourceAdding.size() + childEffects.size());
} else {
m_sources.reserve(m_sources.size() + childEffects.size());
}
+1 -1
View File
@@ -1759,7 +1759,7 @@ ADE_INDEXER(l_UserInterface_Cutscenes,
if (!ade_get_args(L, "*s", &name))
return ade_set_error(L, "o", l_TechRoomCutscene.Set(cutscene_info_h(-1)));
// coverity[uninit_use_in_call] - name is assigned via ade_get_args
// coverity[uninit_use_in_call:FALSE] - name is assigned via ade_get_args
int idx = get_cutscene_index_by_name(name);
if (idx < 0) {
+7
View File
@@ -771,6 +771,8 @@ thruster_bank* thrusterbank_h::Get() const
{
if (!isValid())
return nullptr;
// coverity[returned_null:FALSE] - isValid() specifically checks for modelh.Get() returning null
return &modelh.Get()->thrusters[thrusterbank_index];
}
bool thrusterbank_h::isValid() const
@@ -891,6 +893,8 @@ glow_point_bank* glowpointbank_h::Get() const
{
if (!isValid())
return nullptr;
// coverity[returned_null:FALSE] - isValid() specifically checks for modelh.Get() returning null
return &modelh.Get()->glow_point_banks[glowpointbank_index];
}
bool glowpointbank_h::isValid() const
@@ -1114,6 +1118,7 @@ ADE_FUNC(__len, l_ModelDockingbays, NULL, "Retrieves the number of dockingbays o
if (!dbhp->isValid())
return ade_set_error(L, "i", 0);
// coverity[returned_null:FALSE] - isValid() specifically checks for model_get() returning null
return ade_set_args(L, "i", dbhp->Get()->n_docks);
}
@@ -1127,6 +1132,7 @@ bool dockingbay_h::isValid() const
if (!modelh.isValid())
return false;
// coverity[returned_null:FALSE] - isValid() specifically checks for model_get() returning null
return dock_id >= 0 && dock_id < modelh.Get()->n_docks;
}
dock_bay* dockingbay_h::getDockingBay() const
@@ -1134,6 +1140,7 @@ dock_bay* dockingbay_h::getDockingBay() const
if (!isValid())
return nullptr;
// coverity[returned_null:FALSE] - isValid() specifically checks for model_get() returning null
return &modelh.Get()->docking_bays[dock_id];
}
+3 -3
View File
@@ -137,9 +137,9 @@ void script_parse_lua_script(const char *filename) {
script_function func;
func.language = SC_LUA;
func.function = function;
func.function = std::move(function);
Script_system.AddGameInitFunction(func);
Script_system.AddGameInitFunction(std::move(func));
} catch (const LuaException& e) {
LuaError(Script_system.GetLuaSession(), "Failed to parse %s: %s", filename, e.what());
}
@@ -686,7 +686,7 @@ void script_state::ParseChunkSub(script_function& script_func, const char* debug
auto function = LuaFunction::createFromCode(LuaState, source, function_name);
function.setErrorFunction(LuaFunction::createFromCFunction(LuaState, ade_friendly_error));
script_func.function = function;
script_func.function = std::move(function);
} catch (const LuaException& e) {
LuaError(GetLuaSession(), "%s", e.what());
}
+1 -1
View File
@@ -74,7 +74,7 @@ json_t* LuaValueSerializer::tableToJsonArray(const luacpp::LuaTable& table)
if (!table.getValue(i, arrayVal)) {
json_array_append_new(jsonArray, json_null());
} else {
LuaValueSerializer valueSerializer(arrayVal);
LuaValueSerializer valueSerializer(std::move(arrayVal));
json_array_append_new(jsonArray, valueSerializer.toJson());
}
}
+1 -1
View File
@@ -290,7 +290,7 @@ const float FULLY_TARGETABLE = (viewer_has_primitive_sensors ? ((dist
if (test > Awacs[idx].subsys->awacs_radius)
continue;
// coverity[dead_error_line] - closest_index will be a value other than -1 on future loop iterations
// coverity[dead_error_line:FALSE] - closest_index will be a value other than -1 on future loop iterations
if ((closest_index == -1) || (test < closest))
{
closest = test;
+10 -12
View File
@@ -3211,12 +3211,12 @@ static void parse_ship_values(ship_info* sip, const bool is_template, const bool
if (optional_string("$Default Team:")) {
char temp[NAME_LENGTH];
stuff_string(temp, F_NAME, NAME_LENGTH);
SCP_string name = temp;
if (!stricmp(temp, "none")) {
sip->uses_team_colors = true;
} else {
SCP_string name = temp;
if (Team_Colors.find(name) != Team_Colors.end()) {
sip->default_team_name = name;
sip->default_team_name = std::move(name);
sip->uses_team_colors = true;
} else {
Warning(LOCATION, "Team name %s is invalid. Teams must be defined in colors.tbl.\n", temp);
@@ -11043,11 +11043,9 @@ int ship_create(matrix* orient, vec3d* pos, int ship_type, const char* ship_name
for (int bank = 0; bank < pm->n_glow_point_banks; bank++) {
glow_point_bank_override* gpo = nullptr;
if (sip) {
SCP_unordered_map<int, void*>::iterator gpoi = sip->glowpoint_bank_override_map.find(bank);
if (gpoi != sip->glowpoint_bank_override_map.end()) {
gpo = (glow_point_bank_override*)sip->glowpoint_bank_override_map[bank];
}
SCP_unordered_map<int, void*>::iterator gpoi = sip->glowpoint_bank_override_map.find(bank);
if (gpoi != sip->glowpoint_bank_override_map.end()) {
gpo = (glow_point_bank_override*)sip->glowpoint_bank_override_map[bank];
}
if (gpo) {
@@ -11197,11 +11195,9 @@ static void ship_model_change(int n, int ship_type)
for (int bank = 0; bank < pm->n_glow_point_banks; bank++) {
glow_point_bank_override* gpo = nullptr;
if (sip) {
SCP_unordered_map<int, void*>::iterator gpoi = sip->glowpoint_bank_override_map.find(bank);
if (gpoi != sip->glowpoint_bank_override_map.end()) {
gpo = (glow_point_bank_override*)sip->glowpoint_bank_override_map[bank];
}
SCP_unordered_map<int, void*>::iterator gpoi = sip->glowpoint_bank_override_map.find(bank);
if (gpoi != sip->glowpoint_bank_override_map.end()) {
gpo = (glow_point_bank_override*)sip->glowpoint_bank_override_map[bank];
}
if (gpo) {
@@ -14547,6 +14543,8 @@ int ship_info_lookup(const char *token)
{
// chop off right parenthesis (it exists because otherwise the left wouldn't have been flagged)
char *p2 = strchr(temp2, ')');
if (!p2)
return -1;
*p2 = '\0';
// assemble using hash
+6 -6
View File
@@ -207,7 +207,7 @@ void parse_rank_table(const char* filename)
continue;
}
}
rank_p->promotion_text[persona] = buf;
rank_p->promotion_text[persona] = std::move(buf);
}
if (rank_p->promotion_text.find(-1) == rank_p->promotion_text.end())
@@ -339,7 +339,7 @@ void parse_traitor_tbl(const char* filename)
continue;
}
}
Traitor.debriefing_text[persona] = text;
Traitor.debriefing_text[persona] = std::move(text);
}
if (optional_string("$Voice:"))
@@ -368,12 +368,12 @@ void parse_traitor_tbl(const char* filename)
stuff_string(rec, F_MULTITEXT);
traitor_override_t traitor;
traitor.name = name;
traitor.text = text;
traitor.recommendation_text = rec;
traitor.name = std::move(name);
traitor.text = std::move(text);
traitor.recommendation_text = std::move(rec);
strcpy_s(traitor.voice_filename, file);
Traitor_overrides.push_back(traitor);
Traitor_overrides.push_back(std::move(traitor));
}
}
}
+1 -1
View File
@@ -2574,7 +2574,7 @@ void beam_get_binfo(beam *b, float accuracy, int num_shots, int burst_seed, floa
vm_vec_random_in_circle(&pos1, &center, &orient, 1.f, bwi->t5info.start_pos == Type5BeamPos::RANDOM_OUTSIDE);
if (bwi->t5info.end_pos != Type5BeamPos::CENTER)
vm_vec_random_in_circle(&pos2, &center, &orient, 1.f, bwi->t5info.start_pos == Type5BeamPos::RANDOM_OUTSIDE);
vm_vec_random_in_circle(&pos2, &center, &orient, 1.f, bwi->t5info.end_pos == Type5BeamPos::RANDOM_OUTSIDE);
if (bwi->t5info.no_translate || bwi->t5info.end_pos == Type5BeamPos::SAME_RANDOM)
pos2 = pos1;
+2 -2
View File
@@ -3487,7 +3487,7 @@ int parse_weapon(int subtype, bool replace, const char *filename)
temp_data.ssm_entry = fname;
if (Delayed_SSM_data.find(wip->name) == Delayed_SSM_data.end())
Delayed_SSM_names.push_back(wip->name);
Delayed_SSM_data[wip->name] = temp_data;
Delayed_SSM_data[wip->name] = std::move(temp_data);
} else {
// We'll still want to validate the index later. -MageKing17
delayed_ssm_index_data temp_data;
@@ -3495,7 +3495,7 @@ int parse_weapon(int subtype, bool replace, const char *filename)
temp_data.linenum = get_line_num();
if (Delayed_SSM_indices_data.find(wip->name) == Delayed_SSM_indices_data.end())
Delayed_SSM_indices.push_back(wip->name);
Delayed_SSM_indices_data[wip->name] = temp_data;
Delayed_SSM_indices_data[wip->name] = std::move(temp_data);
}
}// SSM index -Bobboau