Skip to content

Commit

Permalink
Merge pull request scp-fs2open#6260 from Goober5000/coverity_fixes
Browse files Browse the repository at this point in the history 
some coverity fixes
  • Loading branch information
@Goober5000
Goober5000 authored Aug 11, 2024
2 parents 61e9463 + 734efbd commit 8dc1ffe
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 50 deletions.
26 changes: 8 additions & 18 deletions code/missionui/missionscreencommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1661,8 +1661,7 @@ void draw_model_icon(int model_id, uint64_t flags, float closeup_zoom, int x, in
gr_reset_clip();
}

void light_set_all_relevent();
void draw_model_rotating(model_render_params *render_info, int model_id, int x1, int y1, int x2, int y2, float *rotation_buffer, vec3d *closeup_pos, float closeup_zoom, float rev_rate, uint64_t flags, int resize_mode, int effect)
void draw_model_rotating(model_render_params *render_info, int model_id, int x1, int y1, int x2, int y2, float *rotation_buffer, const vec3d *closeup_pos, float closeup_zoom, float rev_rate, uint64_t flags, int resize_mode, int effect)
{
//WMC - Can't draw a non-model
if (model_id < 0)
Expand All @@ -1672,6 +1671,11 @@ void draw_model_rotating(model_render_params *render_info, int model_id, int x1,
angles rot_angles, view_angles;
matrix model_orient;

auto pm = model_get(model_id);
vec3d closeup_pos_default = { { { 0.0f, 0.0f, -(pm->rad * 1.5f) } } };
if (!closeup_pos || IS_VEC_NULL(closeup_pos))
closeup_pos = &closeup_pos_default;

const bool& shadow_disable_override = flags & MR_IS_MISSILE ? Shadow_disable_overrides.disable_mission_select_weapons : Shadow_disable_overrides.disable_mission_select_ships;

if (effect == 2) { // FS2 Effect; Phase 0 Expand scanline, Phase 1 scan the grid and wireframe, Phase 2 scan up and reveal the ship, Phase 3 tilt the camera, Phase 4 start rotating the ship
Expand Down Expand Up @@ -1713,7 +1717,6 @@ void draw_model_rotating(model_render_params *render_info, int model_id, int x1,
ship_normal.xyz.x = 0.0f;
ship_normal.xyz.y = -1.0f;
ship_normal.xyz.z = 0.0f;
polymodel *pm = model_get(model_id);

//Make the clipping plane
float clip = -pm->rad*0.7f;
Expand All @@ -1726,13 +1729,7 @@ void draw_model_rotating(model_render_params *render_info, int model_id, int x1,
vm_vec_scale_sub(&plane_point,&vmd_zero_vector,&wire_normal,clip);

g3_start_frame(1);
if ( (closeup_pos != NULL) && (vm_vec_mag(closeup_pos) > 0.0f) ) {
g3_set_view_matrix(closeup_pos, &vmd_identity_matrix, closeup_zoom);
} else {
vec3d pos = { { { 0.0f, 0.0f, -(pm->rad * 1.5f) } } };
g3_set_view_matrix(&pos, &vmd_identity_matrix, closeup_zoom);
}

g3_set_view_matrix(closeup_pos, &vmd_identity_matrix, closeup_zoom);
gr_set_proj_matrix(Proj_fov, gr_screen.clip_aspect, Min_draw_distance, Max_draw_distance);
gr_set_view_matrix(&Eye_position, &Eye_matrix);

Expand Down Expand Up @@ -1891,15 +1888,8 @@ void draw_model_rotating(model_render_params *render_info, int model_id, int x1,

g3_start_frame(1);

polymodel *pm = model_get(model_id);

// render the wodel
if ( (closeup_pos != NULL) && (vm_vec_mag(closeup_pos) > 0.0f) ) {
g3_set_view_matrix(closeup_pos, &vmd_identity_matrix, closeup_zoom);
} else {
vec3d pos = { { { 0.0f, 0.0f, -(pm->rad * 1.5f) } } };
g3_set_view_matrix(&pos, &vmd_identity_matrix, closeup_zoom);
}
g3_set_view_matrix(closeup_pos, &vmd_identity_matrix, closeup_zoom);

//setup lights
common_setup_room_lights();
Expand Down
2 changes: 1 addition & 1 deletion code/missionui/missionscreencommon.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ int restore_wss_data(ubyte *data);

class ship_info;
void draw_model_icon(int model_id, uint64_t flags, float closeup_zoom, int x1, int x2, int y1, int y2, ship_info* sip = NULL, int resize_mode = GR_RESIZE_FULL, const vec3d *closeup_pos = &vmd_zero_vector);
void draw_model_rotating(model_render_params *render_info, int model_id, int x1, int y1, int x2, int y2, float *rotation_buffer, vec3d *closeup_pos=NULL, float closeup_zoom = .65f, float rev_rate = REVOLUTION_RATE, uint64_t flags = MR_AUTOCENTER | MR_NO_FOGGING, int resize_mode=GR_RESIZE_FULL, int effect = 2);
void draw_model_rotating(model_render_params *render_info, int model_id, int x1, int y1, int x2, int y2, float *rotation_buffer, const vec3d *closeup_pos=nullptr, float closeup_zoom = .65f, float rev_rate = REVOLUTION_RATE, uint64_t flags = MR_AUTOCENTER | MR_NO_FOGGING, int resize_mode=GR_RESIZE_FULL, int effect = 2);

void common_set_team_pointers(int team);
void common_reset_team_pointers();
Expand Down
2 changes: 2 additions & 0 deletions code/options/OptionsManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,14 @@ std::unique_ptr<json_t> OptionsManager::getValueFromConfig(const SCP_string& key
if (override_iter != _config_overrides.end()) {
// We return a reference to an existing object so we need to increment the reference count
json_incref(override_iter->second.get());
// coverity[multiple_init_smart_ptr:FALSE] - according to m!m, this is most likely a false positive: "I think Coverity does not understand the usage of default_delete" in jansson.h, line 23
return std::unique_ptr<json_t>(override_iter->second.get());
}

auto changed_iter = _changed_values.find(key);
if (changed_iter != _changed_values.end()) {
json_incref(changed_iter->second.get());
// coverity[multiple_init_smart_ptr:FALSE] - according to m!m, this is most likely a false positive: "I think Coverity does not understand the usage of default_delete" in jansson.h, line 23
return std::unique_ptr<json_t>(changed_iter->second.get());
}

Expand Down
120 changes: 94 additions & 26 deletions code/parse/sexp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2354,8 +2354,9 @@ int check_sexp_syntax(int node, int return_type, int recursive, int *bad_node, s
// variables should only be typechecked.
if ((Sexp_nodes[node].type & SEXP_FLAG_VARIABLE) && (type != OPF_VARIABLE_NAME)) {
var_index = sexp_get_variable_index(node);
Assert(var_index != -1);

if (var_index < 0)
return SEXP_CHECK_INVALID_VARIABLE;

if (!check_variable_data_type(type,
Sexp_variables[var_index].type,
get_operator_const(op_node),
Expand Down Expand Up @@ -3840,9 +3841,8 @@ int check_sexp_syntax(int node, int return_type, int recursive, int *bad_node, s

case OPF_VARIABLE_NAME:
var_index = sexp_get_variable_index(node);
if ( var_index == -1) {
if (var_index < 0)
return SEXP_CHECK_INVALID_VARIABLE;
}

switch (Operators[op].value)
{
Expand Down Expand Up @@ -13745,6 +13745,11 @@ void sexp_close_sound_from_file(int n)
if (n >= 0)
{
sexp_var = sexp_get_variable_index(n);
if (sexp_var < 0)
{
Warning(LOCATION, "close-sound-from-file: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}

if (!(Sexp_variables[sexp_var].type & SEXP_VARIABLE_NUMBER))
{
Expand Down Expand Up @@ -13805,6 +13810,11 @@ void sexp_play_sound_from_file(int n)
if (n >= 0)
{
sexp_var = sexp_get_variable_index(n);
if (sexp_var < 0)
{
Warning(LOCATION, "play-sound-from-file: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}

if (!(Sexp_variables[sexp_var].type & SEXP_VARIABLE_NUMBER))
{
Expand Down Expand Up @@ -13858,6 +13868,11 @@ void sexp_pause_sound_from_file(int n)
if (n >= 0)
{
sexp_var = sexp_get_variable_index(n);
if (sexp_var < 0)
{
Warning(LOCATION, "pause-sound-from-file: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}

if (!(Sexp_variables[sexp_var].type & SEXP_VARIABLE_NUMBER))
{
Expand Down Expand Up @@ -15825,8 +15840,11 @@ void sexp_set_wing_formation(int node)
for (int n = CDDR(node); n >= 0; n = CDR(n))
{
auto wingp = eval_wing(n);
wingp->formation = formation;
wingp->formation_scale = factor;
if (wingp)
{
wingp->formation = formation;
wingp->formation_scale = factor;
}
}
}

Expand Down Expand Up @@ -16134,6 +16152,11 @@ void sexp_add_background_bitmap(int n, bool is_sun, bool uses_correct_angles)

// ripped from sexp_modify_variable()
sexp_var = sexp_get_variable_index(n);
if (sexp_var < 0)
{
Warning(LOCATION, "sexp-add-%s-bitmap: Variable %s does not exist!", is_sun ? "sun" : "background", Sexp_nodes[n].text);
return;
}

if (Sexp_variables[sexp_var].type & SEXP_VARIABLE_NUMBER)
{
Expand Down Expand Up @@ -24339,11 +24362,16 @@ void sexp_int_to_string(int n)

// get sexp_variable index
sexp_variable_index = sexp_get_variable_index(n);
if (sexp_variable_index < 0)
{
Warning(LOCATION, "int-to-string: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}

// check variable type
if (!(Sexp_variables[sexp_variable_index].type & SEXP_VARIABLE_STRING))
{
Warning(LOCATION, "Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
Warning(LOCATION, "int-to-string: Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
return;
}

Expand Down Expand Up @@ -24374,11 +24402,16 @@ void sexp_string_concatenate(int n)

// get sexp_variable index
sexp_variable_index = sexp_get_variable_index(n);
if (sexp_variable_index < 0)
{
Warning(LOCATION, "string-concatenate: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}

// check variable type
if (!(Sexp_variables[sexp_variable_index].type & SEXP_VARIABLE_STRING))
{
Warning(LOCATION, "Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
Warning(LOCATION, "string-concatenate: Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
return;
}

Expand All @@ -24389,7 +24422,7 @@ void sexp_string_concatenate(int n)
// check length
if (strlen(new_text) >= TOKEN_LENGTH)
{
Warning(LOCATION, "Concatenated string '%s' has " SIZE_T_ARG " characters, but the maximum is %d. The string will be truncated.", new_text, strlen(new_text), TOKEN_LENGTH - 1);
Warning(LOCATION, "string-concatenate: Concatenated string '%s' has " SIZE_T_ARG " characters, but the maximum is %d. The string will be truncated.", new_text, strlen(new_text), TOKEN_LENGTH - 1);
new_text[TOKEN_LENGTH] = 0;
}

Expand All @@ -24409,12 +24442,17 @@ void sexp_string_concatenate_block(int n)

// get sexp_variable index
sexp_variable_index = sexp_get_variable_index(n);
if (sexp_variable_index < 0)
{
Warning(LOCATION, "string-concatenate-block: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}
n = CDR(n);

// check variable type
if (!(Sexp_variables[sexp_variable_index].type & SEXP_VARIABLE_STRING))
{
Warning(LOCATION, "Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
Warning(LOCATION, "string-concatenate-block: Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
return;
}

Expand All @@ -24428,7 +24466,7 @@ void sexp_string_concatenate_block(int n)
// check length
if (new_text.length() >= TOKEN_LENGTH)
{
Warning(LOCATION, "Concatenated string '%s' has " SIZE_T_ARG " characters, but the maximum is %d. The string will be truncated.", new_text.c_str(), new_text.length(), TOKEN_LENGTH - 1);
Warning(LOCATION, "string-concatenate-block: Concatenated string '%s' has " SIZE_T_ARG " characters, but the maximum is %d. The string will be truncated.", new_text.c_str(), new_text.length(), TOKEN_LENGTH - 1);
new_text.resize(TOKEN_LENGTH - 1);
}

Expand Down Expand Up @@ -24462,11 +24500,16 @@ void sexp_string_get_substring(int node)

// get sexp_variable index
sexp_variable_index = sexp_get_variable_index(n);
if (sexp_variable_index < 0)
{
Warning(LOCATION, "string-get-substring: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}

// check variable type
if (!(Sexp_variables[sexp_variable_index].type & SEXP_VARIABLE_STRING))
{
Warning(LOCATION, "Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
Warning(LOCATION, "string-get-substring: Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
return;
}

Expand Down Expand Up @@ -24527,11 +24570,16 @@ void sexp_string_set_substring(int node)

// get sexp_variable index
sexp_variable_index = sexp_get_variable_index(n);
if (sexp_variable_index < 0)
{
Warning(LOCATION, "string-set-substring: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}

// check variable type
if (!(Sexp_variables[sexp_variable_index].type & SEXP_VARIABLE_STRING))
{
Warning(LOCATION, "Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
Warning(LOCATION, "string-set-substring: Cannot assign a string to a non-string variable %s!", Sexp_variables[sexp_variable_index].variable_name);
return;
}

Expand Down Expand Up @@ -24566,12 +24614,12 @@ void sexp_string_set_substring(int node)

// This shouldn't happen
Assertion(substring_begin_byte < substring_end_byte,
"The begin position of the substring must be less than the end position!");
"string-set-substring: The begin position of the substring must be less than the end position!");

new_text.replace(substring_begin_byte, substring_end_byte - substring_begin_byte, new_substring);

if (new_text.size() >= TOKEN_LENGTH) {
Warning(LOCATION, "Concatenated string is too long and will be truncated.");
Warning(LOCATION, "string-set-substring: Concatenated string is too long and will be truncated.");

new_text.resize(TOKEN_LENGTH - 1);

Expand Down Expand Up @@ -24600,10 +24648,15 @@ void sexp_modify_variable_xstr(int n)

// get sexp_variable index
auto sexp_variable_index = sexp_get_variable_index(n);
if (sexp_variable_index < 0)
{
Warning(LOCATION, "modify-variable-xstr: Variable %s does not exist!", Sexp_nodes[n].text);
return;
}
n = CDR(n);

if (!(Sexp_variables[sexp_variable_index].type & SEXP_VARIABLE_STRING)) {
Warning(LOCATION, "Variable for modify-variable-xstr has to be a string variable!");
Warning(LOCATION, "modify-variable-xstr: Variable %s must be a string variable!", Sexp_variables[sexp_variable_index].variable_name);
return;
}

Expand Down Expand Up @@ -26313,9 +26366,13 @@ int sexp_script_eval(int node, int return_type, bool concat_args = false)
{
int variable_index = sexp_get_variable_index(n);

if (!(Sexp_variables[variable_index].type & SEXP_VARIABLE_STRING))
if (variable_index < 0)
{
Warning(LOCATION, "Variable for script-eval has to be a string variable!");
Warning(LOCATION, "script-eval: Variable %s does not exist!", Sexp_nodes[n].text);
}
else if (!(Sexp_variables[variable_index].type & SEXP_VARIABLE_STRING))
{
Warning(LOCATION, "script-eval: Variable %s must be a string variable!", Sexp_variables[variable_index].variable_name);
}
else if (ret != nullptr)
{
Expand Down Expand Up @@ -34337,6 +34394,9 @@ const char *sexp_error_message(int num)
case SEXP_CHECK_AMBIGUOUS_EVENT_NAME:
return "Ambiguous event name (more than one event with the same name)";

case SEXP_CHECK_INVALID_CONTAINER:
return "Invalid container name";

case SEXP_CHECK_MISSING_CONTAINER_MODIFIER:
return "Missing container modifier";

Expand Down Expand Up @@ -34754,9 +34814,8 @@ int check_dynamic_value_node_type(int node, bool is_string, bool is_number)

if (Sexp_nodes[node].type & SEXP_FLAG_VARIABLE) {
const int var_index = sexp_get_variable_index(node);
Assertion(var_index != -1,
"Attempt to get index of invalid variable %s. Please report!",
Sexp_nodes[node].text);
if (var_index < 0)
return SEXP_CHECK_INVALID_VARIABLE;
const int var_type = Sexp_variables[var_index].type;
if (((var_type & SEXP_VARIABLE_STRING) && !is_string) || ((var_type & SEXP_VARIABLE_NUMBER) && !is_number)) {
return SEXP_CHECK_INVALID_VARIABLE_TYPE;
Expand All @@ -34768,9 +34827,8 @@ int check_dynamic_value_node_type(int node, bool is_string, bool is_number)
}
} else if (Sexp_nodes[node].subtype == SEXP_ATOM_CONTAINER_DATA) {
const auto *p_container = get_sexp_container(Sexp_nodes[node].text);
Assertion(p_container,
"Attempt to check dynamic value node type of data of nonexistent container %s. Please report!",
Sexp_nodes[node].text);
if (!p_container)
return SEXP_CHECK_INVALID_CONTAINER;
if (!check_container_data_sexp_arg_type(p_container->type, is_string, is_number)) {
return SEXP_CHECK_WRONG_CONTAINER_DATA_TYPE;
}
Expand All @@ -34796,7 +34854,11 @@ void sexp_modify_variable(int n)
// get sexp_variable index
sexp_variable_index = sexp_get_variable_index(n);

if (Sexp_variables[sexp_variable_index].type & SEXP_VARIABLE_NUMBER)
if (sexp_variable_index < 0)
{
Warning(LOCATION, "modify-variable: Variable %s does not exist!", Sexp_nodes[n].text);
}
else if (Sexp_variables[sexp_variable_index].type & SEXP_VARIABLE_NUMBER)
{
// get new numerical value
new_number = eval_sexp(CDR(n));
Expand All @@ -34815,7 +34877,7 @@ void sexp_modify_variable(int n)
}
else
{
Error(LOCATION, "Invalid variable type.\n");
Warning(LOCATION, "modify-variable: Variable %s has an unknown type!", Sexp_variables[sexp_variable_index].variable_name);
}
}

Expand Down Expand Up @@ -34971,6 +35033,12 @@ void sexp_copy_variable_from_index(int node)
// now get the variable we are modifying
to_index = sexp_get_variable_index(CDR(node));

if (to_index < 0)
{
Warning(LOCATION, "copy-variable-from-index: Variable %s does not exist!", Sexp_nodes[CDR(node)].text);
return;
}

// verify matching types
if ( ((Sexp_variables[from_index].type & SEXP_VARIABLE_NUMBER) && !(Sexp_variables[to_index].type & SEXP_VARIABLE_NUMBER))
|| ((Sexp_variables[from_index].type & SEXP_VARIABLE_STRING) && !(Sexp_variables[to_index].type & SEXP_VARIABLE_STRING)) )
Expand Down
Loading

0 comments on commit 8dc1ffe

Please sign in to comment.