PVS-Studio report

[alphrixus]

Tested versions

https://github.com/godotengine/godot/releases/tag/4.2.2-stable

System information

.

Issue description

I found some bugs in the code using the PVS-Studio analyzer and wrote a short article about it.

Steps to reproduce

.

Minimal reproduction project (MRP)

.

[AThousandShips]

Please add details about actionable problems here on the report, instead of on an off site, and double check the analysis is actually valid, your article isn't very easy to navigate

[bruvzg]

Quickly looked through, will look in depth tomorrow:

Fragments N1-N2:

https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/servers/rendering/shader_language.cpp#L40 https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/scene/gui/item_list.cpp#L655 relevant for current master, should be fixed to avoid future issues, but all existing uses of these macros are fine.

Fragment N3:

https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/servers/rendering/renderer_rd/storage_rd/material_storage.cpp#L932 https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/drivers/gles3/storage/material_storage.cpp#L1004 still present, might be actual issue (or callback check is sufficient in all real use cases).

Fragment N4:

https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/scene/resources/font.cpp#L2023 typo in the warning message, still present, similar code is used 6 times, so probably can be moved to function.

Fragment N5:

• https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/modules/gridmap/editor/grid_map_editor_plugin.cpp#L832 redundant mb->is_pressed()
• https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/modules/gridmap/editor/grid_map_editor_plugin.cpp#L836 redundant mb->is_pressed()
• https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/scene/3d/physics/character_body_3d.cpp#L581-L583 redundant !r_state.floor
• https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/platform/linuxbsd/x11/display_server_x11.cpp#L2109-L2110 probably should be !wd_parent.is_popup.
• https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/modules/gdscript/gdscript_analyzer.cpp#L991 redundant && member.variable->initializer

Fragment N6

• https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/modules/gridmap/editor/grid_map_editor_plugin.cpp#L258 node null check is redundant.
• https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/modules/gdscript/gdscript_analyzer.cpp#L4807 seems like a missing && p_ternary_op->true_expr.
• node_3d_editor_plugin.cpp:4100, already fixed.
• project_export.cpp:950, already fixed.
• https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/editor/editor_node.cpp#L1218-L1220 title_bar null check is redundant.
• https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/editor/project_manager.cpp#L1034-L1036 same
• https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/drivers/gles3/rasterizer_canvas_gles3.cpp#L2177 missing ERR_FAIL_NULL(render_target); check.
• https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/core/variant/dictionary.cpp#L229-L244 not sure what's the logic here, either redundant, or we should check p_from._p as well.
• https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/core/extension/extension_api_dump.cpp#L1242 class_doc null check is redundant.

Fragment N7

https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/core/templates/local_vector.h#L296-L310 in general, yes, but this operator should not be used for anything except Packed*Array for sending to GPU (all trivially constructable). consexpr check similar to CowData can be added.

Fragment N8

• https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/modules/gdscript/editor/gdscript_highlighter.cpp#L379-L381 still present, String is null terminated so should not cause any real issues.

Fragment N9

https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/drivers/gles3/storage/particles_storage.cpp#L958-L964 still present.

Fragment N10

https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/editor/plugins/animation_state_machine_editor.cpp#L684-L691 still present, probably should be delete_window->get_cancel_button()->set_disabled(!delete_tree->get_next_selected(nullptr));, code added in #24402

Fragment N11

https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/core/string/locales.h#L1113 copy-paste from Unicode web page, display only string.

Fragment N12

https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/drivers/gles3/storage/mesh_storage.cpp#L1301 https://github.com/godotengine/godot/blob/92212946532450494683929b82c450e95851a0bf/drivers/gles3/storage/mesh_storage.cpp#L1415 probably should be RS::ARRAY_FORMAT_VERTEX | RS::ARRAY_FORMAT_NORMAL | RS::ARRAY_FORMAT_TANGENT;, cc @clayjohn (introduced in #85092)

Fragment N13

https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/core/io/image.cpp#L2148 https://github.com/godotengine/godot/blob/03afb92efa18874da19f7fc185a32c005d20aa1d/core/io/image.cpp#L2174 still present

Fragment N14

Seems to be fixed. This code significantly changed, and no longer locking/unlocking directly.