Merge pull request #29306 from qarmin/small_code_fixes

Small fixes to unrechable code, possibly overflows, using NULL pointers

core/hash_map.h

@@ -162,20 +162,21 @@ private:
 			new_hash_table[i] = 0;
 		}
 
-		for (int i = 0; i < (1 << hash_table_power); i++) {
+		if (hash_table) {
+			for (int i = 0; i < (1 << hash_table_power); i++) {
 
-			while (hash_table[i]) {
+				while (hash_table[i]) {
 
-				Element *se = hash_table[i];
-				hash_table[i] = se->next;
-				int new_pos = se->hash & ((1 << new_hash_table_power) - 1);
-				se->next = new_hash_table[new_pos];
-				new_hash_table[new_pos] = se;
+					Element *se = hash_table[i];
+					hash_table[i] = se->next;
+					int new_pos = se->hash & ((1 << new_hash_table_power) - 1);
+					se->next = new_hash_table[new_pos];
+					new_hash_table[new_pos] = se;
+				}
 			}
-		}
 
-		if (hash_table)
 			memdelete_arr(hash_table);
+		}
 		hash_table = new_hash_table;
 		hash_table_power = new_hash_table_power;
 	}

core/io/config_file.cpp

@@ -198,10 +198,6 @@ Error ConfigFile::load(const String &p_path) {
 			section = next_tag.name;
 		}
 	}
-
-	memdelete(f);
-
-	return OK;
 }
 
 void ConfigFile::_bind_methods() {

core/project_settings.cpp

@@ -579,10 +579,6 @@ Error ProjectSettings::_load_settings_text(const String p_path) {
 			section = next_tag.name;
 		}
 	}
-
-	memdelete(f);
-
-	return OK;
 }
 
 Error ProjectSettings::_load_settings_text_or_binary(const String p_text_path, const String p_bin_path) {

core/ustring.cpp

@@ -2956,26 +2956,12 @@ String String::replace(const char *p_key, const char *p_with) const {
 
 String String::replace_first(const String &p_key, const String &p_with) const {
 
-	String new_string;
-	int search_from = 0;
-	int result = 0;
-
-	while ((result = find(p_key, search_from)) >= 0) {
-
-		new_string += substr(search_from, result - search_from);
-		new_string += p_with;
-		search_from = result + p_key.length();
-		break;
+	int pos = find(p_key);
+	if (pos >= 0) {
+		return substr(0, pos) + p_with + substr(pos + p_key.length(), length());
 	}
 
-	if (search_from == 0) {
-
-		return *this;
-	}
-
-	new_string += substr(search_from, length() - search_from);
-
-	return new_string;
+	return *this;
 }
 String String::replacen(const String &p_key, const String &p_with) const {
 

drivers/gles2/rasterizer_scene_gles2.cpp

@@ -2261,7 +2261,7 @@ void RasterizerSceneGLES2::_render_render_list(RenderList::Element **p_elements,
 		bool rebind_reflection = false;
 		bool rebind_lightmap = false;
 
-		if (!p_shadow) {
+		if (!p_shadow && material->shader) {
 
 			bool unshaded = material->shader->spatial.unshaded;
 
@@ -2281,7 +2281,7 @@ void RasterizerSceneGLES2::_render_render_list(RenderList::Element **p_elements,
 
 			bool depth_prepass = false;
 
-			if (!p_alpha_pass && material->shader && material->shader->spatial.depth_draw_mode == RasterizerStorageGLES2::Shader::Spatial::DEPTH_DRAW_ALPHA_PREPASS) {
+			if (!p_alpha_pass && material->shader->spatial.depth_draw_mode == RasterizerStorageGLES2::Shader::Spatial::DEPTH_DRAW_ALPHA_PREPASS) {
 				depth_prepass = true;
 			}
 
@@ -2919,7 +2919,7 @@ void RasterizerSceneGLES2::render_scene(const Transform &p_cam_transform, const
 	if (storage->frame.current_rt && state.used_screen_texture) {
 		//copy screen texture
 
-		if (storage->frame.current_rt && storage->frame.current_rt->multisample_active) {
+		if (storage->frame.current_rt->multisample_active) {
 			// Resolve framebuffer to front buffer before copying
 #ifdef GLES_OVER_GL
 

drivers/gles3/rasterizer_scene_gles3.cpp

@@ -4552,8 +4552,8 @@ void RasterizerSceneGLES3::render_scene(const Transform &p_cam_transform, const
 	}
 
 	_post_process(env, p_cam_projection);
-
-	if (false && shadow_atlas) {
+	// Needed only for debugging
+	/*	if (shadow_atlas && storage->frame.current_rt) {
 
 		//_copy_texture_to_front_buffer(shadow_atlas->depth);
 		storage->canvas->canvas_begin();
@@ -4563,7 +4563,7 @@ void RasterizerSceneGLES3::render_scene(const Transform &p_cam_transform, const
 		storage->canvas->draw_generic_textured_rect(Rect2(0, 0, storage->frame.current_rt->width / 2, storage->frame.current_rt->height / 2), Rect2(0, 0, 1, 1));
 	}
 
-	if (false && storage->frame.current_rt) {
+	if (storage->frame.current_rt) {
 
 		//_copy_texture_to_front_buffer(shadow_atlas->depth);
 		storage->canvas->canvas_begin();
@@ -4573,7 +4573,7 @@ void RasterizerSceneGLES3::render_scene(const Transform &p_cam_transform, const
 		storage->canvas->draw_generic_textured_rect(Rect2(0, 0, storage->frame.current_rt->width / 16, storage->frame.current_rt->height / 16), Rect2(0, 0, 1, 1));
 	}
 
-	if (false && reflection_atlas && storage->frame.current_rt) {
+	if (reflection_atlas && storage->frame.current_rt) {
 
 		//_copy_texture_to_front_buffer(shadow_atlas->depth);
 		storage->canvas->canvas_begin();
@@ -4582,7 +4582,7 @@ void RasterizerSceneGLES3::render_scene(const Transform &p_cam_transform, const
 		storage->canvas->draw_generic_textured_rect(Rect2(0, 0, storage->frame.current_rt->width / 2, storage->frame.current_rt->height / 2), Rect2(0, 0, 1, 1));
 	}
 
-	if (false && directional_shadow.fbo) {
+	if (directional_shadow.fbo) {
 
 		//_copy_texture_to_front_buffer(shadow_atlas->depth);
 		storage->canvas->canvas_begin();
@@ -4592,7 +4592,7 @@ void RasterizerSceneGLES3::render_scene(const Transform &p_cam_transform, const
 		storage->canvas->draw_generic_textured_rect(Rect2(0, 0, storage->frame.current_rt->width / 2, storage->frame.current_rt->height / 2), Rect2(0, 0, 1, 1));
 	}
 
-	if (false && env_radiance_tex) {
+	if ( env_radiance_tex) {
 
 		//_copy_texture_to_front_buffer(shadow_atlas->depth);
 		storage->canvas->canvas_begin();
@@ -4603,8 +4603,7 @@ void RasterizerSceneGLES3::render_scene(const Transform &p_cam_transform, const
 		storage->canvas->draw_generic_textured_rect(Rect2(0, 0, storage->frame.current_rt->width / 2, storage->frame.current_rt->height / 2), Rect2(0, 0, 1, 1));
 		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_LINEAR);
 		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
-	}
-
+	}*/
 	//disable all stuff
 }
 

editor/audio_stream_preview.cpp

@@ -129,7 +129,7 @@ void AudioStreamPreviewGenerator::_preview_thread(void *p_preview) {
 			float max = -1000;
 			float min = 1000;
 			int from = uint64_t(i) * to_read / to_write;
-			int to = uint64_t(i + 1) * to_read / to_write;
+			int to = (uint64_t(i) + 1) * to_read / to_write;
 			to = MIN(to, to_read);
 			from = MIN(from, to_read - 1);
 			if (to == from) {

editor/code_editor.cpp

@@ -930,7 +930,7 @@ void CodeTextEditor::convert_case(CaseStyle p_case) {
 	for (int i = begin; i <= end; i++) {
 		int len = text_editor->get_line(i).length();
 		if (i == end)
-			len -= len - end_col;
+			len = end_col;
 		if (i == begin)
 			len -= begin_col;
 		String new_line = text_editor->get_line(i).substr(i == begin ? begin_col : 0, len);

editor/editor_node.cpp

@@ -5044,7 +5044,7 @@ void EditorNode::_feature_profile_changed() {
 		main_editor_buttons[EDITOR_3D]->set_visible(!profile->is_feature_disabled(EditorFeatureProfile::FEATURE_3D));
 		main_editor_buttons[EDITOR_SCRIPT]->set_visible(!profile->is_feature_disabled(EditorFeatureProfile::FEATURE_SCRIPT));
 		main_editor_buttons[EDITOR_ASSETLIB]->set_visible(!profile->is_feature_disabled(EditorFeatureProfile::FEATURE_ASSET_LIB));
-		if (profile->is_feature_disabled(EditorFeatureProfile::FEATURE_3D) || profile->is_feature_disabled(EditorFeatureProfile::FEATURE_ASSET_LIB) || profile->is_feature_disabled(EditorFeatureProfile::FEATURE_ASSET_LIB)) {
+		if (profile->is_feature_disabled(EditorFeatureProfile::FEATURE_3D) || profile->is_feature_disabled(EditorFeatureProfile::FEATURE_SCRIPT) || profile->is_feature_disabled(EditorFeatureProfile::FEATURE_ASSET_LIB)) {
 			_editor_select(EDITOR_2D);
 		}
 	} else {

editor/plugins/editor_preview_plugins.cpp

@@ -643,7 +643,7 @@ Ref<Texture> EditorAudioStreamPreviewPlugin::generate(const RES &p_from, const S
 		float max = -1000;
 		float min = 1000;
 		int from = uint64_t(i) * frame_length / w;
-		int to = uint64_t(i + 1) * frame_length / w;
+		int to = (uint64_t(i) + 1) * frame_length / w;
 		to = MIN(to, frame_length);
 		from = MIN(from, frame_length - 1);
 		if (to == from) {

editor/scene_tree_editor.cpp

@@ -399,15 +399,17 @@ bool SceneTreeEditor::_add_nodes(Node *p_node, TreeItem *p_parent) {
 
 void SceneTreeEditor::_node_visibility_changed(Node *p_node) {
 
-	if (p_node != get_scene_node() && !p_node->get_owner()) {
+	if (!p_node || (p_node != get_scene_node() && !p_node->get_owner())) {
 
 		return;
 	}
-	TreeItem *item = p_node ? _find(tree->get_root(), p_node->get_path()) : NULL;
-	if (!item) {
 
+	TreeItem *item = _find(tree->get_root(), p_node->get_path());
+
+	if (!item) {
 		return;
 	}
+
 	int idx = item->get_button_by_id(0, BUTTON_VISIBILITY);
 	ERR_FAIL_COND(idx == -1);
 

modules/gdscript/gdscript_parser.cpp

@@ -1995,7 +1995,6 @@ GDScriptParser::Node *GDScriptParser::_reduce_expression(Node *p_node, bool p_to
 				}
 			}
 
-			ERR_FAIL_V(op);
 		} break;
 		default: {
 			return p_node;

modules/recast/navigation_mesh_editor_plugin.cpp

@@ -67,9 +67,7 @@ void NavigationMeshEditor::_bake_pressed() {
 	EditorNavigationMeshGenerator::get_singleton()->clear(node->get_navigation_mesh());
 	EditorNavigationMeshGenerator::get_singleton()->bake(node->get_navigation_mesh(), node);
 
-	if (node) {
-		node->update_gizmo();
-	}
+	node->update_gizmo();
 }
 
 void NavigationMeshEditor::_clear_pressed() {

modules/webm/video_stream_webm.cpp

@@ -413,10 +413,11 @@ void VideoStreamPlaybackWebm::delete_pointers() {
 
 	if (audio_frame)
 		memdelete(audio_frame);
-	for (int i = 0; i < video_frames_capacity; ++i)
-		memdelete(video_frames[i]);
-	if (video_frames)
+	if (video_frames) {
+		for (int i = 0; i < video_frames_capacity; ++i)
+			memdelete(video_frames[i]);
 		memfree(video_frames);
+	}
 
 	if (video)
 		memdelete(video);

platform/x11/joypad_linux.cpp

@@ -444,10 +444,10 @@ InputDefault::JoyAxis JoypadLinux::axis_correct(const input_absinfo *p_abs, int
 		jx.min = -1;
 		if (p_value < 0) {
 			jx.value = (float)-p_value / min;
+		} else {
+			jx.value = (float)p_value / max;
 		}
-		jx.value = (float)p_value / max;
-	}
-	if (min == 0) {
+	} else if (min == 0) {
 		jx.min = 0;
 		jx.value = 0.0f + (float)p_value / max;
 	}

scene/2d/camera_2d.cpp

@@ -44,15 +44,16 @@ void Camera2D::_update_scroll() {
 		return;
 	}
 
+	if (!viewport)
+		return;
+
 	if (current) {
 
 		ERR_FAIL_COND(custom_viewport && !ObjectDB::get_instance(custom_viewport_id));
 
 		Transform2D xform = get_camera_transform();
 
-		if (viewport) {
-			viewport->set_canvas_transform(xform);
-		}
+		viewport->set_canvas_transform(xform);
 
 		Size2 screen_size = viewport->get_visible_rect().size;
 		Point2 screen_offset = (anchor_mode == ANCHOR_MODE_DRAG_CENTER ? (screen_size * 0.5) : Point2());

scene/animation/animation_blend_tree.cpp

@@ -1049,7 +1049,7 @@ AnimationNodeBlendTree::ConnectionError AnimationNodeBlendTree::can_connect_node
 		return CONNECTION_ERROR_NO_INPUT;
 	}
 
-	if (!nodes.has(p_input_node)) {
+	if (p_input_node == p_output_node) {
 		return CONNECTION_ERROR_SAME_NODE;
 	}
 

scene/gui/tree.cpp

@@ -318,7 +318,7 @@ void TreeItem::set_custom_draw(int p_column, Object *p_object, const StringName
 
 void TreeItem::set_collapsed(bool p_collapsed) {
 
-	if (collapsed == p_collapsed)
+	if (collapsed == p_collapsed || !tree)
 		return;
 	collapsed = p_collapsed;
 	TreeItem *ci = tree->selected_item;
@@ -344,8 +344,7 @@ void TreeItem::set_collapsed(bool p_collapsed) {
 	}
 
 	_changed_notify();
-	if (tree)
-		tree->emit_signal("item_collapsed", this);
+	tree->emit_signal("item_collapsed", this);
 }
 
 bool TreeItem::is_collapsed() {