Fix Steam input "crc" errors, and some other Coverity reports of uninitialized scalar variable #91454

akien-mga · 2024-05-02T11:13:30Z

Going through a few reports on https://scan.coverity.com/projects/godotengine-godot?tab=overview

Lots of low hanging fruits there to improve the code quality and find potential bugs.

The input mapping change fixes these issues:

akien-mga · 2024-05-02T11:18:30Z

drivers/gles3/storage/light_storage.h

@@ -141,7 +141,7 @@ struct ReflectionAtlas {
 	int size = 0;

 	int mipmap_count = 1; // number of mips, including original
-	int mipmap_size[8];
+	int mipmap_size[8] = {};


Fixes this:

CC @clayjohn @BastiaanOlij

There's a bunch more issues with uninitialized stuff in rendering code, would be good for the rendering team to make a pass on the reports and decide whether to init all the structs properly, or what other approach to take. We've had a number of bugs on some platforms/GPUs due to uninitialized struct members.

akien-mga · 2024-05-02T11:19:18Z

drivers/gles3/rasterizer_gles3.cpp

+	} else {
+		ERR_FAIL_MSG(vformat("GL ERROR: Invalid or unhandled source '%d' in debug callback.", source));


These fix these errors:

CC @lawnjelly

Although I'm in the blame this is because I'm registered as initial author for 4.x, but this was copy pasted from 3.x GLES2, which is karrofel it looks like.

But fix looks fine to me. 👍

akien-mga · 2024-05-02T11:19:36Z

drivers/gles3/rasterizer_gles3.cpp

-	} else if (type == _EXT_DEBUG_TYPE_PERFORMANCE_ARB) {
-		strcpy(debType, "Performance");
-	} else if (type == _EXT_DEBUG_TYPE_OTHER_ARB) {
-		strcpy(debType, "Other");


Removed as the early return makes it dead code.

akien-mga · 2024-05-02T11:21:47Z

modules/gdscript/language_server/godot_lsp.h

@@ -236,7 +236,7 @@ struct ReferenceContext {
 	/**
 	 * Include the declaration of the current symbol.
 	 */
-	bool includeDeclaration;
+	bool includeDeclaration = false;


Fixes this:

But this is actually never set anywhere, so the code where it was being used is bogus.

It was added in #80973, CC @ryanabx.

akien-mga · 2024-05-02T11:22:12Z

scene/resources/2d/tile_set.h

+		int bits[TileSet::CELL_NEIGHBOR_MAX] = {};
+		bool is_valid_bit[TileSet::CELL_NEIGHBOR_MAX] = {};


Fixes this:

CC @groud

akien-mga · 2024-05-02T11:24:35Z

core/input/input.cpp

@@ -1495,6 +1495,7 @@ void Input::parse_mapping(const String &p_mapping) {
 		JoyAxis output_axis = _get_output_axis(output);
 		if (output_button == JoyButton::INVALID && output_axis == JoyAxis::INVALID) {
 			print_verbose(vformat("Unrecognized output string \"%s\" in mapping:\n%s", output, p_mapping));
+			continue;


This fixes what I reported here: #73224 (comment)

CC @rsubtil

Yes, it makes sense, I didn't notice that detail when changing the error message to a warning 👍

I believe this should also fix these issues:

[Steam] [Linux] XBox Series X controller shows CRC errors #88630

Parse_mapping error on Steam version only #92578

akien-mga · 2024-05-02T11:25:19Z

drivers/gles3/rasterizer_canvas_gles3.h

+			float z_far = 0.0;
+			float y_offset = 0.0;


CC @clayjohn @BastiaanOlij

I'm not sure about these. To me it seems totally pointless to initialize certain variables when we don't know if they will be used. The trouble is that we assign an RID before configuring the light. So the Coverity scan doesn't have enough information here to be really useful.

I removed these changes from the PR for now.

akien-mga · 2024-05-02T11:26:18Z

thirdparty/enet/enet/godot.h

+   uint8_t host[16] = {};
+   uint16_t port = 0;
+   uint8_t wildcard = 0;


This is C code so my fix was invalid, would need a different approach. Removed from this PR for now.

akien-mga · 2024-05-02T11:28:08Z

editor/debugger/debug_adapter/debug_adapter_types.h

-	int variablesReference;
-	bool expensive;
+	int variablesReference = 0;
+	bool expensive = false;


Fixes this type of errors:

CC @rsubtil

Good catch, expensive ended up never being set in the code, so this was actually causing undefined behavior.

I can confirm it should be false as AFAIK Godot still doesn't support partial/pagination behavior to requesting variables from stack dumps, so there's no point in hinting such behavior to DAP when Godot will already have all that information.

rsubtil · 2024-05-04T09:50:25Z

editor/debugger/debug_adapter/debug_adapter_types.h

 	String format;
-	bool sendTelemetry = false; // Just in case :)
-	bool showUser;
+	bool sendTelemetry = false;


There goes my little easter egg 😅

Brought it back ;)

…itialized scalar variable - Fixes godotengine#88630. - Fixes godotengine#92578.

akien-mga added bug enhancement topic:codestyle labels May 2, 2024

akien-mga added this to the 4.3 milestone May 2, 2024

akien-mga requested review from a team as code owners May 2, 2024 11:13

akien-mga force-pushed the coverity-checks branch from 64d757c to 70d0d8b Compare May 2, 2024 11:16

akien-mga commented May 2, 2024

View reviewed changes

akien-mga force-pushed the coverity-checks branch from 70d0d8b to 7e056c2 Compare May 2, 2024 11:29

akien-mga mentioned this pull request May 2, 2024

Change message of unknown joypad property from error to verbose #73224

Merged

rsubtil reviewed May 4, 2024

View reviewed changes

akien-mga modified the milestones: 4.3, 4.4 May 29, 2024

akien-mga force-pushed the coverity-checks branch from 7e056c2 to 39b0ed1 Compare May 31, 2024 07:50

akien-mga modified the milestones: 4.4, 4.3 May 31, 2024

akien-mga changed the title ~~Fix various Coverity reports of uninitialized scalar variable~~ Fix various Coverity reports of uninitialized scalar variable, fix Steam input CRC errors May 31, 2024

Fix Steam input "crc" errors, and some other Coverity reports of unin…

62120c7

…itialized scalar variable - Fixes godotengine#88630. - Fixes godotengine#92578.

akien-mga force-pushed the coverity-checks branch from 39b0ed1 to 62120c7 Compare May 31, 2024 07:55

akien-mga changed the title ~~Fix various Coverity reports of uninitialized scalar variable, fix Steam input CRC errors~~ Fix Steam input "crc" errors, and some other Coverity reports of uninitialized scalar variable May 31, 2024

akien-mga added topic:input cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:codestyle cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release and removed topic:codestyle labels May 31, 2024

akien-mga merged commit 41e762c into godotengine:master Jun 3, 2024
16 checks passed

akien-mga deleted the coverity-checks branch June 3, 2024 08:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Steam input "crc" errors, and some other Coverity reports of uninitialized scalar variable #91454

Fix Steam input "crc" errors, and some other Coverity reports of uninitialized scalar variable #91454

akien-mga commented May 2, 2024 •

edited

Loading

akien-mga May 2, 2024

akien-mga May 2, 2024

lawnjelly May 4, 2024

akien-mga May 2, 2024

akien-mga May 2, 2024

akien-mga May 2, 2024

akien-mga May 2, 2024

rsubtil May 4, 2024

akien-mga May 31, 2024

akien-mga May 2, 2024

clayjohn May 2, 2024

akien-mga May 31, 2024

akien-mga May 2, 2024

akien-mga May 2, 2024

akien-mga May 2, 2024

rsubtil May 4, 2024

rsubtil May 4, 2024

akien-mga May 31, 2024

		} else {
		ERR_FAIL_MSG(vformat("GL ERROR: Invalid or unhandled source '%d' in debug callback.", source));

		int bits[TileSet::CELL_NEIGHBOR_MAX] = {};
		bool is_valid_bit[TileSet::CELL_NEIGHBOR_MAX] = {};

Fix Steam input "crc" errors, and some other Coverity reports of uninitialized scalar variable #91454

Fix Steam input "crc" errors, and some other Coverity reports of uninitialized scalar variable #91454

Conversation

akien-mga commented May 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga commented May 2, 2024 •

edited

Loading