Merge pull request #31077 from qarmin/coverity_bugs

Change some code proposed by Coverity and Cppcheck

core/io/resource_loader.cpp

@@ -940,8 +940,7 @@ bool ResourceLoader::add_custom_resource_format_loader(String script_path) {
 	ERR_EXPLAIN("Cannot instance script as custom resource loader, expected 'ResourceFormatLoader' inheritance, got: " + String(ibt));
 	ERR_FAIL_COND_V(obj == NULL, false);
 
-	ResourceFormatLoader *crl = NULL;
+	ResourceFormatLoader *crl = Object::cast_to<ResourceFormatLoader>(obj);
-	crl = Object::cast_to<ResourceFormatLoader>(obj);
 	crl->set_script(s.get_ref_ptr());
 	ResourceLoader::add_resource_format_loader(crl);
 

core/io/resource_saver.cpp

@@ -222,8 +222,7 @@ bool ResourceSaver::add_custom_resource_format_saver(String script_path) {
 	ERR_EXPLAIN("Cannot instance script as custom resource saver, expected 'ResourceFormatSaver' inheritance, got: " + String(ibt));
 	ERR_FAIL_COND_V(obj == NULL, false);
 
-	ResourceFormatSaver *crl = NULL;
+	ResourceFormatSaver *crl = Object::cast_to<ResourceFormatSaver>(obj);
-	crl = Object::cast_to<ResourceFormatSaver>(obj);
 	crl->set_script(s.get_ref_ptr());
 	ResourceSaver::add_resource_format_saver(crl);
 

core/pool_allocator.cpp

@@ -539,6 +539,10 @@ void PoolAllocator::unlock(ID p_mem) {
 		return;
 	mt_lock();
 	Entry *e = get_entry(p_mem);
+	if (!e) {
+		mt_unlock();
+		ERR_FAIL_COND(!e);
+	}
 	if (e->lock == 0) {
 		mt_unlock();
 		ERR_PRINT("e->lock == 0");

drivers/gles3/rasterizer_storage_gles3.cpp

@@ -2522,8 +2522,8 @@ _FORCE_INLINE_ static void _fill_std140_variant_ubo_value(ShaderLanguage::DataTy
 
 			int v = value;
 			GLuint *gui = (GLuint *)data;
-			gui[0] = v & 1 ? GL_TRUE : GL_FALSE;
+			gui[0] = (v & 1) ? GL_TRUE : GL_FALSE;
-			gui[1] = v & 2 ? GL_TRUE : GL_FALSE;
+			gui[1] = (v & 2) ? GL_TRUE : GL_FALSE;
 
 		} break;
 		case ShaderLanguage::TYPE_BVEC3: {

drivers/pulseaudio/audio_driver_pulseaudio.cpp

@@ -266,7 +266,10 @@ Error AudioDriverPulseAudio::init() {
 	}
 
 	while (pa_ready == 0) {
-		pa_mainloop_iterate(pa_ml, 1, NULL);
+		ret = pa_mainloop_iterate(pa_ml, 1, NULL);
+		if (ret < 0) {
+			ERR_PRINT("pa_mainloop_iterate error");
+		}
 	}
 
 	if (pa_ready < 0) {

editor/collada/collada.cpp

@@ -1696,7 +1696,7 @@ Collada::Node *Collada::_parse_visual_scene_node(XMLParser &parser) {
 					}
 				}
 
-			} else if (section == "node") {
+			} else {
 
 				/* Found a child node!! what to do..*/
 

editor/editor_inspector.cpp

@@ -378,7 +378,7 @@ bool EditorPropertyRevert::get_instanced_node_original_property(Node *p_node, co
 		node = node->get_owner();
 	}
 
-	if (!found) {
+	if (!found && node) {
 		//if not found, try default class value
 		Variant attempt = ClassDB::class_get_default_property_value(node->get_class_name(), p_prop);
 		if (attempt.get_type() != Variant::NIL) {
@@ -1302,6 +1302,7 @@ void EditorInspector::remove_inspector_plugin(const Ref<EditorInspectorPlugin> &
 		}
 	}
 
+	ERR_FAIL_COND(idx == -1);
 	for (int i = idx; i < inspector_plugin_count - 1; i++) {
 		inspector_plugins[i] = inspector_plugins[i + 1];
 	}

editor/editor_plugin.cpp

@@ -326,7 +326,8 @@ void EditorPlugin::remove_autoload_singleton(const String &p_name) {
 
 Ref<ConfigFile> EditorPlugin::get_config() {
 	Ref<ConfigFile> cf = memnew(ConfigFile);
-	cf->load(_dir_cache.plus_file("plugin.cfg"));
+	Error err = cf->load(_dir_cache.plus_file("plugin.cfg"));
+	ERR_FAIL_COND_V(err != OK, cf);
 	return cf;
 }
 

editor/editor_settings.cpp

@@ -773,10 +773,16 @@ void EditorSettings::create() {
 
 	if (d->file_exists(exe_path + "/._sc_")) {
 		self_contained = true;
-		extra_config->load(exe_path + "/._sc_");
+		Error err = extra_config->load(exe_path + "/._sc_");
+		if (err != OK) {
+			ERR_PRINTS("Can't load config from path: " + exe_path + "/._sc_");
+		}
 	} else if (d->file_exists(exe_path + "/_sc_")) {
 		self_contained = true;
-		extra_config->load(exe_path + "/_sc_");
+		Error err = extra_config->load(exe_path + "/_sc_");
+		if (err != OK) {
+			ERR_PRINTS("Can't load config from path: " + exe_path + "/_sc_");
+		}
 	}
 	memdelete(d);
 
@@ -1208,9 +1214,12 @@ String EditorSettings::get_feature_profiles_dir() const {
 void EditorSettings::set_project_metadata(const String &p_section, const String &p_key, Variant p_data) {
 	Ref<ConfigFile> cf = memnew(ConfigFile);
 	String path = get_project_settings_dir().plus_file("project_metadata.cfg");
-	cf->load(path);
+	Error err;
+	err = cf->load(path);
+	ERR_FAIL_COND(err != OK);
 	cf->set_value(p_section, p_key, p_data);
-	cf->save(path);
+	err = cf->save(path);
+	ERR_FAIL_COND(err != OK);
 }
 
 Variant EditorSettings::get_project_metadata(const String &p_section, const String &p_key, Variant p_default) const {

editor/plugin_config_dialog.cpp

@@ -130,7 +130,8 @@ void PluginConfigDialog::_notification(int p_what) {
 void PluginConfigDialog::config(const String &p_config_path) {
 	if (p_config_path.length()) {
 		Ref<ConfigFile> cf = memnew(ConfigFile);
-		cf->load(p_config_path);
+		Error err = cf->load(p_config_path);
+		ERR_FAIL_COND(err != OK);
 
 		name_edit->set_text(cf->get_value("plugin", "name", ""));
 		subfolder_edit->set_text(p_config_path.get_base_dir().get_basename().get_file());

modules/mono/csharp_script.cpp

@@ -2913,11 +2913,10 @@ Variant CSharpScript::_new(const Variant **p_args, int p_argcount, Variant::Call
 
 	r_error.error = Variant::CallError::CALL_OK;
 	REF ref;
-	Object *owner = NULL;
 
 	ERR_FAIL_NULL_V(native, Variant());
 
-	owner = ClassDB::instance(NATIVE_GDMONOCLASS_NAME(native));
+	Object *owner = ClassDB::instance(NATIVE_GDMONOCLASS_NAME(native));
 
 	Reference *r = Object::cast_to<Reference>(owner);
 	if (r) {

modules/visual_script/visual_script_property_selector.cpp

@@ -130,6 +130,7 @@ void VisualScriptPropertySelector::_update_search() {
 		{
 			String b = String(E->get());
 			category = search_options->create_item(root);
+			if (category) {
 				category->set_text(0, b.replace_first("*", ""));
 				category->set_selectable(0, false);
 				Ref<Texture> icon;
@@ -137,6 +138,7 @@ void VisualScriptPropertySelector::_update_search() {
 				icon = EditorNode::get_singleton()->get_class_icon(rep);
 				category->set_icon(0, icon);
 			}
+		}
 		if (properties || seq_connect) {
 			if (instance) {
 				instance->get_property_list(&props, true);

scene/2d/sprite.h

@@ -110,7 +110,7 @@ public:
 	void set_frame(int p_frame);
 	int get_frame() const;
 
-	void set_frame_coords(const Vector2 &p_row);
+	void set_frame_coords(const Vector2 &p_coord);
 	Vector2 get_frame_coords() const;
 
 	void set_vframes(int p_amount);

scene/3d/sprite_3d.h

@@ -173,7 +173,7 @@ public:
 	void set_frame(int p_frame);
 	int get_frame() const;
 
-	void set_frame_coords(const Vector2 &p_row);
+	void set_frame_coords(const Vector2 &p_coord);
 	Vector2 get_frame_coords() const;
 
 	void set_vframes(int p_amount);

scene/animation/animation_blend_space_1d.cpp

@@ -266,7 +266,7 @@ float AnimationNodeBlendSpace1D::process(float p_time, bool p_seek) {
 
 	// fill in weights
 
-	if (point_lower == -1) {
+	if (point_lower == -1 && point_higher != -1) {
 		// we are on the left side, no other point to the left
 		// we just play the next point.
 

scene/animation/animation_blend_space_2d.cpp

@@ -527,7 +527,7 @@ float AnimationNodeBlendSpace2D::process(float p_time, bool p_seek) {
 			}
 		}
 
-		if (new_closest != closest) {
+		if (new_closest != closest && new_closest != -1) {
 
 			float from = 0;
 			if (blend_mode == BLEND_MODE_DISCRETE_CARRY && closest != -1) {

scene/gui/control.cpp

@@ -1772,17 +1772,6 @@ void Control::set_global_position(const Point2 &p_point, bool p_keep_margins) {
 	set_position(inv.xform(p_point), p_keep_margins);
 }
 
-Rect2 Control::_compute_child_rect(const float p_anchors[4], const float p_margins[4]) const {
-
-	Rect2 anchorable = get_parent_anchorable_rect();
-	Rect2 result = anchorable;
-	for (int i = 0; i < 4; i++) {
-		result.grow_margin((Margin)i, p_anchors[i] * anchorable.get_size()[i % 2] + p_margins[i]);
-	}
-
-	return result;
-}
-
 void Control::_compute_anchors(Rect2 p_rect, const float p_margins[4], float (&r_anchors)[4]) {
 
 	Size2 parent_rect_size = get_parent_anchorable_rect().size;

scene/gui/control.h

@@ -230,7 +230,6 @@ private:
 	void _update_scroll();
 	void _resize(const Size2 &p_size);
 
-	Rect2 _compute_child_rect(const float p_anchors[4], const float p_margins[4]) const;
 	void _compute_margins(Rect2 p_rect, const float p_anchors[4], float (&r_margins)[4]);
 	void _compute_anchors(Rect2 p_rect, const float p_margins[4], float (&r_anchors)[4]);
 

scene/main/node.cpp

@@ -1021,7 +1021,7 @@ void Node::_validate_child_name(Node *p_child, bool p_force_human_readable) {
 
 		if (!unique) {
 
-			node_hrcr_count.ref();
+			ERR_FAIL_COND(!node_hrcr_count.ref());
 			String name = "@" + String(p_child->get_name()) + "@" + itos(node_hrcr_count.get());
 			p_child->data.name = name;
 		}
@@ -2198,8 +2198,11 @@ void Node::_duplicate_and_reown(Node *p_new_parent, const Map<Node *, Node *> &p
 		ERR_EXPLAIN("Node: Could not duplicate: " + String(get_class()));
 		ERR_FAIL_COND(!obj);
 		node = Object::cast_to<Node>(obj);
-		if (!node)
+		if (!node) {
 			memdelete(obj);
+			ERR_EXPLAIN("Node: Could not duplicate: " + String(get_class()));
+			ERR_FAIL();
+		}
 	}
 
 	List<PropertyInfo> plist;
@@ -2295,16 +2298,16 @@ Node *Node::duplicate_and_reown(const Map<Node *, Node *> &p_reown_map) const {
 
 	ERR_FAIL_COND_V(get_filename() != "", NULL);
 
-	Node *node = NULL;
-
 	Object *obj = ClassDB::instance(get_class());
 	ERR_EXPLAIN("Node: Could not duplicate: " + String(get_class()));
 	ERR_FAIL_COND_V(!obj, NULL);
-	node = Object::cast_to<Node>(obj);
-	if (!node)
-		memdelete(obj);
-	ERR_FAIL_COND_V(!node, NULL);
 
+	Node *node = Object::cast_to<Node>(obj);
+	if (!node) {
+		memdelete(obj);
+		ERR_EXPLAIN("Node: Could not duplicate: " + String(get_class()));
+		ERR_FAIL_V(NULL);
+	}
 	node->set_name(get_name());
 
 	List<PropertyInfo> plist;

scene/resources/dynamic_font.cpp

@@ -494,7 +494,7 @@ DynamicFontAtSize::Character DynamicFontAtSize::_bitmap_to_character(FT_Bitmap b
 						int byte = i * bitmap.pitch + (j >> 3);
 						int bit = 1 << (7 - (j % 8));
 						wr[ofs + 0] = 255; //grayscale as 1
-						wr[ofs + 1] = bitmap.buffer[byte] & bit ? 255 : 0;
+						wr[ofs + 1] = (bitmap.buffer[byte] & bit) ? 255 : 0;
 					} break;
 					case FT_PIXEL_MODE_GRAY:
 						wr[ofs + 0] = 255; //grayscale as 1

scene/resources/font.cpp

@@ -133,6 +133,7 @@ PoolVector<int> BitmapFont::_get_chars() const {
 	while ((key = char_map.next(key))) {
 
 		const Character *c = char_map.getptr(*key);
+		ERR_FAIL_COND_V(!c, PoolVector<int>());
 		chars.push_back(*key);
 		chars.push_back(c->texture_idx);
 		chars.push_back(c->rect.position.x);

scene/resources/theme.h

@@ -139,7 +139,7 @@ public:
 	static void set_default(const Ref<Theme> &p_default);
 
 	static Ref<Theme> get_project_default();
-	static void set_project_default(const Ref<Theme> &p_default);
+	static void set_project_default(const Ref<Theme> &p_project_default);
 
 	static void set_default_icon(const Ref<Texture> &p_icon);
 	static void set_default_style(const Ref<StyleBox> &p_style);

servers/physics/space_sw.cpp

@@ -348,11 +348,9 @@ bool PhysicsDirectSpaceStateSW::collide_shape(RID p_shape, const Transform &p_sh
 	cbk.max = p_result_max;
 	cbk.amount = 0;
 	cbk.ptr = r_results;
-	CollisionSolverSW::CallbackResult cbkres = NULL;
+	CollisionSolverSW::CallbackResult cbkres = PhysicsServerSW::_shape_col_cbk;
 
-	PhysicsServerSW::CollCbkData *cbkptr = NULL;
+	PhysicsServerSW::CollCbkData *cbkptr = &cbk;
-	cbkptr = &cbk;
-	cbkres = PhysicsServerSW::_shape_col_cbk;
 
 	for (int i = 0; i < amount; i++) {
 

servers/physics_2d/body_2d_sw.cpp

@@ -608,7 +608,7 @@ void Body2DSW::call_queries() {
 			set_force_integration_callback(0, StringName());
 		} else {
 			Variant::CallError ce;
-			if (fi_callback->callback_udata.get_type()) {
+			if (fi_callback->callback_udata.get_type() != Variant::NIL) {
 
 				obj->call(fi_callback->method, vp, 2, ce);
 

servers/physics_2d/space_2d_sw.cpp

@@ -330,11 +330,9 @@ bool Physics2DDirectSpaceStateSW::collide_shape(RID p_shape, const Transform2D &
 	cbk.amount = 0;
 	cbk.passed = 0;
 	cbk.ptr = r_results;
-	CollisionSolver2DSW::CallbackResult cbkres = NULL;
+	CollisionSolver2DSW::CallbackResult cbkres = Physics2DServerSW::_shape_col_cbk;
 
-	Physics2DServerSW::CollCbkData *cbkptr = NULL;
+	Physics2DServerSW::CollCbkData *cbkptr = &cbk;
-	cbkptr = &cbk;
-	cbkres = Physics2DServerSW::_shape_col_cbk;
 
 	for (int i = 0; i < amount; i++) {
 

servers/visual/visual_server_viewport.cpp

@@ -85,6 +85,7 @@ void VisualServerViewport::_draw_viewport(Viewport *p_viewport, ARVRInterface::E
 	if (!p_viewport->hide_canvas && !p_viewport->disable_environment && VSG::scene->scenario_owner.owns(p_viewport->scenario)) {
 
 		VisualServerScene::Scenario *scenario = VSG::scene->scenario_owner.get(p_viewport->scenario);
+		ERR_FAIL_COND(!scenario);
 		if (VSG::scene_render->is_environment(scenario->environment)) {
 			scenario_draw_canvas_bg = VSG::scene_render->environment_get_background(scenario->environment) == VS::ENV_BG_CANVAS;