From e6deba8d196a206ff350bc4d9fff783f78395d33 Mon Sep 17 00:00:00 2001 From: Crazy-P Date: Sat, 21 Apr 2018 22:35:23 +0800 Subject: [PATCH] Fixes logically dead code (Coverity) Fixes reported logically dead codes by Coverity * image.cpp: Doesn't really need any modification. But to remove the bug report then we have to move the MAX call away from the for loop statement. * rasterizer_gles3.cpp: Removes unnecessary elif condition since it is checked earlier in the function * collada.cpp: If stamement never reached due to macro ERR_CONTINUE does the same. * navigation_mesh.cpp: Variables should always be null - however, also checked for the very same condition in their function call. Leaving this for review (whether the function call is necessary or not) * path_editor_plugin.cpp: If cancel is true, then it should restore the edited value to the original provided. http://docs.godotengine.org/en/3.0/classes/class_editorspatialgizmo.html#class-editorspatialgizmo-commit-handle * spatial_editor_gizmos.cpp: the very condition of i >= 3 is predetermined in the if case right before it. Thus case 1 is always '1' and case 2 is always '-1' * grid_map_editor.cpp: Same as above in spatial_editor_gizmos.cpp * voxel_light_baker.cpp: Same as above in spatial_editor_gizmos.cpp * visual_server.cpp: Same as above in spatial_editor_gizmos.cpp * visual_script_expression.cpp: char '-' is already true in the switch case mechanism. Thus it can never reach to default case. * particles.cpp: Case 'PARAM_MAX' is unreachable due to index checking right before the switch execution. * shader_language.cpp: Invalid index is handled in switch default case. `type < TYPE_FLOAT && type > TYPE_VEC4` -> `(type < TYPE_FLOAT || type > TYPE_VEC4`) Fixes the "always false problem" in TODO comment. --- core/image.cpp | 5 ++- core/node_path.cpp | 3 +- drivers/gles3/rasterizer_gles3.cpp | 2 -- editor/collada/collada.cpp | 4 +-- editor/plugins/navigation_mesh_generator.cpp | 34 ++++++++----------- editor/plugins/path_editor_plugin.cpp | 11 ++---- editor/spatial_editor_gizmos.cpp | 4 +-- modules/gridmap/grid_map_editor_plugin.cpp | 4 +-- .../visual_script_expression.cpp | 7 +--- scene/3d/particles.cpp | 6 ---- scene/3d/voxel_light_baker.cpp | 4 +-- servers/visual/shader_language.cpp | 15 +++----- servers/visual_server.cpp | 4 +-- 13 files changed, 38 insertions(+), 65 deletions(-) diff --git a/core/image.cpp b/core/image.cpp index 2ac8ffea56e..9f03892e745 100644 --- a/core/image.cpp +++ b/core/image.cpp @@ -366,6 +366,8 @@ int Image::get_mipmap_count() const { template static void _convert(int p_width, int p_height, const uint8_t *p_src, uint8_t *p_dst) { + uint32_t max_bytes = MAX(read_bytes, write_bytes); + for (int y = 0; y < p_height; y++) { for (int x = 0; x < p_width; x++) { @@ -379,7 +381,8 @@ static void _convert(int p_width, int p_height, const uint8_t *p_src, uint8_t *p rgba[1] = rofs[0]; rgba[2] = rofs[0]; } else { - for (uint32_t i = 0; i < MAX(read_bytes, write_bytes); i++) { + + for (uint32_t i = 0; i < max_bytes; i++) { rgba[i] = (i < read_bytes) ? rofs[i] : 0; } diff --git a/core/node_path.cpp b/core/node_path.cpp index cd7ad775341..64983fc0916 100644 --- a/core/node_path.cpp +++ b/core/node_path.cpp @@ -264,8 +264,9 @@ NodePath NodePath::get_as_property_path() const { Vector new_path = data->subpath; String initial_subname = data->path[0]; + for (size_t i = 1; i < data->path.size(); i++) { - initial_subname += i == 0 ? data->path[i].operator String() : "/" + data->path[i]; + initial_subname += "/" + data->path[i]; } new_path.insert(0, initial_subname); diff --git a/drivers/gles3/rasterizer_gles3.cpp b/drivers/gles3/rasterizer_gles3.cpp index ca39d9f966d..0fb69494f44 100644 --- a/drivers/gles3/rasterizer_gles3.cpp +++ b/drivers/gles3/rasterizer_gles3.cpp @@ -111,8 +111,6 @@ static void GLAPIENTRY _gl_debug_print(GLenum source, GLenum type, GLuint id, GL strcpy(debType, "Portability"); else if (type == _EXT_DEBUG_TYPE_PERFORMANCE_ARB) strcpy(debType, "Performance"); - else if (type == _EXT_DEBUG_TYPE_OTHER_ARB) - strcpy(debType, "Other"); if (severity == _EXT_DEBUG_SEVERITY_HIGH_ARB) strcpy(debSev, "High"); diff --git a/editor/collada/collada.cpp b/editor/collada/collada.cpp index 4ce57d7f63b..734229d014c 100644 --- a/editor/collada/collada.cpp +++ b/editor/collada/collada.cpp @@ -2266,10 +2266,8 @@ void Collada::_merge_skeletons2(VisualScene *p_vscene) { } node = node->parent; } - ERR_CONTINUE(!sk); - if (!sk) - continue; //bleh + ERR_CONTINUE(!sk); if (!skeleton) { skeleton = sk; diff --git a/editor/plugins/navigation_mesh_generator.cpp b/editor/plugins/navigation_mesh_generator.cpp index 5d4e5520f4c..0537c5c31f2 100644 --- a/editor/plugins/navigation_mesh_generator.cpp +++ b/editor/plugins/navigation_mesh_generator.cpp @@ -280,26 +280,20 @@ void NavigationMeshGenerator::bake(Ref p_nav_mesh, Node *p_node) _build_recast_navigation_mesh(p_nav_mesh, &ep, hf, chf, cset, poly_mesh, detail_mesh, vertices, indices); - if (hf) { - rcFreeHeightField(hf); - hf = 0; - } - if (chf) { - rcFreeCompactHeightfield(chf); - chf = 0; - } - if (cset) { - rcFreeContourSet(cset); - cset = 0; - } - if (poly_mesh) { - rcFreePolyMesh(poly_mesh); - poly_mesh = 0; - } - if (detail_mesh) { - rcFreePolyMeshDetail(detail_mesh); - detail_mesh = 0; - } + rcFreeHeightField(hf); + hf = 0; + + rcFreeCompactHeightfield(chf); + chf = 0; + + rcFreeContourSet(cset); + cset = 0; + + rcFreePolyMesh(poly_mesh); + poly_mesh = 0; + + rcFreePolyMeshDetail(detail_mesh); + detail_mesh = 0; } ep.step(TTR("Done!"), 11); } diff --git a/editor/plugins/path_editor_plugin.cpp b/editor/plugins/path_editor_plugin.cpp index 056219a5758..6dde639c549 100644 --- a/editor/plugins/path_editor_plugin.cpp +++ b/editor/plugins/path_editor_plugin.cpp @@ -167,18 +167,12 @@ void PathSpatialGizmo::commit_handle(int p_idx, const Variant &p_restore, bool p Vector3 ofs; - if (p_cancel) { - - return; - } - if (t == 0) { - if (p_cancel) { - c->set_point_in(p_idx, p_restore); return; } + ur->create_action(TTR("Set Curve In Position")); ur->add_do_method(c.ptr(), "set_point_in", idx, c->get_point_in(idx)); ur->add_undo_method(c.ptr(), "set_point_in", idx, p_restore); @@ -186,10 +180,11 @@ void PathSpatialGizmo::commit_handle(int p_idx, const Variant &p_restore, bool p } else { if (p_cancel) { - c->set_point_out(idx, p_restore); + return; } + ur->create_action(TTR("Set Curve Out Position")); ur->add_do_method(c.ptr(), "set_point_out", idx, c->get_point_out(idx)); ur->add_undo_method(c.ptr(), "set_point_out", idx, p_restore); diff --git a/editor/spatial_editor_gizmos.cpp b/editor/spatial_editor_gizmos.cpp index 8c90d86b9e8..a3c4b73ae4c 100644 --- a/editor/spatial_editor_gizmos.cpp +++ b/editor/spatial_editor_gizmos.cpp @@ -4052,9 +4052,9 @@ SpatialEditorGizmos::SpatialEditorGizmos() { for (int k = 0; k < 3; k++) { if (i < 3) - face_points[j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); + face_points[j][(i + k) % 3] = v[k]; else - face_points[3 - j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); + face_points[3 - j][(i + k) % 3] = -v[k]; } } //tri 1 diff --git a/modules/gridmap/grid_map_editor_plugin.cpp b/modules/gridmap/grid_map_editor_plugin.cpp index f523eef8952..4b96824dcab 100644 --- a/modules/gridmap/grid_map_editor_plugin.cpp +++ b/modules/gridmap/grid_map_editor_plugin.cpp @@ -1154,9 +1154,9 @@ GridMapEditor::GridMapEditor(EditorNode *p_editor) { for (int k = 0; k < 3; k++) { if (i < 3) - face_points[j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); + face_points[j][(i + k) % 3] = v[k]; else - face_points[3 - j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); + face_points[3 - j][(i + k) % 3] = -v[k]; } } diff --git a/modules/visual_script/visual_script_expression.cpp b/modules/visual_script/visual_script_expression.cpp index 16de04e4cfb..bf06e7af7f4 100644 --- a/modules/visual_script/visual_script_expression.cpp +++ b/modules/visual_script/visual_script_expression.cpp @@ -455,7 +455,7 @@ Error VisualScriptExpression::_get_token(Token &r_token) { break; } - if (cchar == '-' || (cchar >= '0' && cchar <= '9')) { + if (cchar >= '0' && cchar <= '9') { //a number String num; @@ -466,11 +466,6 @@ Error VisualScriptExpression::_get_token(Token &r_token) { #define READING_DONE 4 int reading = READING_INT; - if (cchar == '-') { - num += '-'; - cchar = GET_CHAR(); - } - CharType c = cchar; bool exp_sign = false; bool exp_beg = false; diff --git a/scene/3d/particles.cpp b/scene/3d/particles.cpp index 693b416f6de..d1bd059b635 100644 --- a/scene/3d/particles.cpp +++ b/scene/3d/particles.cpp @@ -1028,8 +1028,6 @@ void ParticlesMaterial::set_param(Parameter p_param, float p_value) { case PARAM_ANIM_OFFSET: { VisualServer::get_singleton()->material_set_param(_get_material(), shader_names->anim_offset, p_value); } break; - case PARAM_MAX: { - }; } } float ParticlesMaterial::get_param(Parameter p_param) const { @@ -1082,8 +1080,6 @@ void ParticlesMaterial::set_param_randomness(Parameter p_param, float p_value) { case PARAM_ANIM_OFFSET: { VisualServer::get_singleton()->material_set_param(_get_material(), shader_names->anim_offset_random, p_value); } break; - case PARAM_MAX: { - }; } } float ParticlesMaterial::get_param_randomness(Parameter p_param) const { @@ -1160,8 +1156,6 @@ void ParticlesMaterial::set_param_texture(Parameter p_param, const Ref case PARAM_ANIM_OFFSET: { VisualServer::get_singleton()->material_set_param(_get_material(), shader_names->anim_offset_texture, p_texture); } break; - case PARAM_MAX: { - }; } _queue_shader_change(); diff --git a/scene/3d/voxel_light_baker.cpp b/scene/3d/voxel_light_baker.cpp index d389b69ef3f..13700e0bd37 100644 --- a/scene/3d/voxel_light_baker.cpp +++ b/scene/3d/voxel_light_baker.cpp @@ -2338,9 +2338,9 @@ Ref VoxelLightBaker::create_debug_multimesh(DebugMode p_mode) { for (int k = 0; k < 3; k++) { if (i < 3) - face_points[j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); + face_points[j][(i + k) % 3] = v[k]; else - face_points[3 - j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); + face_points[3 - j][(i + k) % 3] = -v[k]; } } diff --git a/servers/visual/shader_language.cpp b/servers/visual/shader_language.cpp index 7aca4978811..01cf689a094 100644 --- a/servers/visual/shader_language.cpp +++ b/servers/visual/shader_language.cpp @@ -2677,7 +2677,6 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons return NULL; } - bool index_valid = false; DataType member_type = TYPE_VOID; switch (expr->get_datatype()) { @@ -2696,7 +2695,7 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons _set_error("Only integer constants are allowed as index at the moment"); return NULL; } - index_valid = true; + switch (expr->get_datatype()) { case TYPE_BVEC2: member_type = TYPE_BOOL; break; case TYPE_VEC2: member_type = TYPE_FLOAT; break; @@ -2721,7 +2720,7 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons _set_error("Only integer constants are allowed as index at the moment"); return NULL; } - index_valid = true; + switch (expr->get_datatype()) { case TYPE_BVEC3: member_type = TYPE_BOOL; break; case TYPE_VEC3: member_type = TYPE_FLOAT; break; @@ -2745,7 +2744,7 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons _set_error("Only integer constants are allowed as index at the moment"); return NULL; } - index_valid = true; + switch (expr->get_datatype()) { case TYPE_BVEC4: member_type = TYPE_BOOL; break; case TYPE_VEC4: member_type = TYPE_FLOAT; break; @@ -2760,11 +2759,6 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons } } - if (!index_valid) { - _set_error("Invalid index"); - return NULL; - } - OperatorNode *op = alloc_node(); op->op = OP_INDEX; op->return_cache = member_type; @@ -3662,7 +3656,8 @@ Error ShaderLanguage::_parse_shader(const Map &p_funct _set_error("void datatype not allowed here"); return ERR_PARSE_ERROR; } - if (!uniform && type < TYPE_FLOAT && type > TYPE_VEC4) { // FIXME: always false! should it be || instead? + + if (!uniform && (type < TYPE_FLOAT || type > TYPE_VEC4)) { _set_error("Invalid type for varying, only float,vec2,vec3,vec4 allowed."); return ERR_PARSE_ERROR; } diff --git a/servers/visual_server.cpp b/servers/visual_server.cpp index f8640720cbb..59dd1ab495a 100644 --- a/servers/visual_server.cpp +++ b/servers/visual_server.cpp @@ -214,9 +214,9 @@ RID VisualServer::_make_test_cube() { for (int k = 0; k < 3; k++) { if (i < 3) - face_points[j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); + face_points[j][(i + k) % 3] = v[k]; else - face_points[3 - j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); + face_points[3 - j][(i + k) % 3] = -v[k]; } normal_points[j] = Vector3(); normal_points[j][i % 3] = (i >= 3 ? -1 : 1);