From 31e7ee63f21e7b86d41cdb724824d4dc0804f281 Mon Sep 17 00:00:00 2001 From: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Date: Sat, 27 Apr 2024 11:56:39 +0200 Subject: [PATCH] Fix unsafe uses of `Callable.is_null()` `Callable.is_null()` is not equivalent to `!Callable.is_valid()` and doesn't guarantee the call is valid. --- core/core_bind.cpp | 2 +- core/object/object.cpp | 4 ++-- core/object/undo_redo.cpp | 4 ++-- drivers/gles3/storage/utilities.cpp | 4 ++-- editor/gui/editor_validation_panel.cpp | 2 +- platform/android/display_server_android.cpp | 2 +- platform/ios/display_server_ios.mm | 2 +- platform/linuxbsd/x11/display_server_x11.cpp | 2 +- platform/macos/display_server_macos.mm | 12 ++++++------ platform/macos/godot_content_view.mm | 2 +- platform/macos/godot_window_delegate.mm | 4 ++-- platform/web/display_server_web.cpp | 10 +++++----- platform/web/javascript_bridge_singleton.cpp | 2 +- platform/windows/display_server_windows.cpp | 10 +++++----- servers/physics_2d/godot_area_2d.h | 4 ++-- servers/physics_3d/godot_area_3d.h | 4 ++-- servers/rendering/renderer_canvas_cull.cpp | 4 ++-- .../rendering/renderer_rd/storage_rd/utilities.cpp | 4 ++-- tests/display_server_mock.h | 2 +- 19 files changed, 40 insertions(+), 40 deletions(-) diff --git a/core/core_bind.cpp b/core/core_bind.cpp index 467b696eae7..0996db9d890 100644 --- a/core/core_bind.cpp +++ b/core/core_bind.cpp @@ -1921,7 +1921,7 @@ void EngineDebugger::send_message(const String &p_msg, const Array &p_data) { Error EngineDebugger::call_capture(void *p_user, const String &p_cmd, const Array &p_data, bool &r_captured) { Callable &capture = *(Callable *)p_user; - if (capture.is_null()) { + if (!capture.is_valid()) { return FAILED; } Variant cmd = p_cmd, data = p_data; diff --git a/core/object/object.cpp b/core/object/object.cpp index f8d2feb5a82..30629ba205a 100644 --- a/core/object/object.cpp +++ b/core/object/object.cpp @@ -1455,7 +1455,7 @@ Error Object::connect(const StringName &p_signal, const Callable &p_callable, ui } bool Object::is_connected(const StringName &p_signal, const Callable &p_callable) const { - ERR_FAIL_COND_V_MSG(p_callable.is_null(), false, "Cannot determine if connected to '" + p_signal + "': the provided callable is null."); + ERR_FAIL_COND_V_MSG(p_callable.is_null(), false, "Cannot determine if connected to '" + p_signal + "': the provided callable is null."); // Should use `is_null`, see note in `connect` about the use of `is_valid`. const SignalData *s = signal_map.getptr(p_signal); if (!s) { bool signal_is_valid = ClassDB::has_signal(get_class_name(), p_signal); @@ -1478,7 +1478,7 @@ void Object::disconnect(const StringName &p_signal, const Callable &p_callable) } bool Object::_disconnect(const StringName &p_signal, const Callable &p_callable, bool p_force) { - ERR_FAIL_COND_V_MSG(p_callable.is_null(), false, "Cannot disconnect from '" + p_signal + "': the provided callable is null."); + ERR_FAIL_COND_V_MSG(p_callable.is_null(), false, "Cannot disconnect from '" + p_signal + "': the provided callable is null."); // Should use `is_null`, see note in `connect` about the use of `is_valid`. SignalData *s = signal_map.getptr(p_signal); if (!s) { diff --git a/core/object/undo_redo.cpp b/core/object/undo_redo.cpp index 6a1385e2682..4d67cd930eb 100644 --- a/core/object/undo_redo.cpp +++ b/core/object/undo_redo.cpp @@ -144,7 +144,7 @@ void UndoRedo::create_action(const String &p_name, MergeMode p_mode, bool p_back } void UndoRedo::add_do_method(const Callable &p_callable) { - ERR_FAIL_COND(p_callable.is_null()); + ERR_FAIL_COND(!p_callable.is_valid()); ERR_FAIL_COND(action_level <= 0); ERR_FAIL_COND((current_action + 1) >= actions.size()); @@ -169,7 +169,7 @@ void UndoRedo::add_do_method(const Callable &p_callable) { } void UndoRedo::add_undo_method(const Callable &p_callable) { - ERR_FAIL_COND(p_callable.is_null()); + ERR_FAIL_COND(!p_callable.is_valid()); ERR_FAIL_COND(action_level <= 0); ERR_FAIL_COND((current_action + 1) >= actions.size()); diff --git a/drivers/gles3/storage/utilities.cpp b/drivers/gles3/storage/utilities.cpp index 7e2e3dfa2bd..356dc06733b 100644 --- a/drivers/gles3/storage/utilities.cpp +++ b/drivers/gles3/storage/utilities.cpp @@ -299,7 +299,7 @@ void Utilities::visibility_notifier_call(RID p_notifier, bool p_enter, bool p_de ERR_FAIL_NULL(vn); if (p_enter) { - if (!vn->enter_callback.is_null()) { + if (vn->enter_callback.is_valid()) { if (p_deferred) { vn->enter_callback.call_deferred(); } else { @@ -307,7 +307,7 @@ void Utilities::visibility_notifier_call(RID p_notifier, bool p_enter, bool p_de } } } else { - if (!vn->exit_callback.is_null()) { + if (vn->exit_callback.is_valid()) { if (p_deferred) { vn->exit_callback.call_deferred(); } else { diff --git a/editor/gui/editor_validation_panel.cpp b/editor/gui/editor_validation_panel.cpp index c08af1915f2..80bb08517c6 100644 --- a/editor/gui/editor_validation_panel.cpp +++ b/editor/gui/editor_validation_panel.cpp @@ -81,7 +81,7 @@ void EditorValidationPanel::set_update_callback(const Callable &p_callback) { } void EditorValidationPanel::update() { - ERR_FAIL_COND(update_callback.is_null()); + ERR_FAIL_COND(!update_callback.is_valid()); if (pending_update) { return; diff --git a/platform/android/display_server_android.cpp b/platform/android/display_server_android.cpp index c6f2f821178..9869756be14 100644 --- a/platform/android/display_server_android.cpp +++ b/platform/android/display_server_android.cpp @@ -326,7 +326,7 @@ void DisplayServerAndroid::window_set_drop_files_callback(const Callable &p_call } void DisplayServerAndroid::_window_callback(const Callable &p_callable, const Variant &p_arg, bool p_deferred) const { - if (!p_callable.is_null()) { + if (p_callable.is_valid()) { if (p_deferred) { p_callable.call_deferred(p_arg); } else { diff --git a/platform/ios/display_server_ios.mm b/platform/ios/display_server_ios.mm index cd6f855d776..62bc55dce83 100644 --- a/platform/ios/display_server_ios.mm +++ b/platform/ios/display_server_ios.mm @@ -218,7 +218,7 @@ void DisplayServerIOS::send_window_event(DisplayServer::WindowEvent p_event) con } void DisplayServerIOS::_window_callback(const Callable &p_callable, const Variant &p_arg) const { - if (!p_callable.is_null()) { + if (p_callable.is_valid()) { p_callable.call(p_arg); } } diff --git a/platform/linuxbsd/x11/display_server_x11.cpp b/platform/linuxbsd/x11/display_server_x11.cpp index 0041b4c7f34..e854fe9274b 100644 --- a/platform/linuxbsd/x11/display_server_x11.cpp +++ b/platform/linuxbsd/x11/display_server_x11.cpp @@ -4992,7 +4992,7 @@ void DisplayServerX11::process_events() { files.write[i] = files[i].replace("file://", "").uri_decode(); } - if (!windows[window_id].drop_files_callback.is_null()) { + if (windows[window_id].drop_files_callback.is_valid()) { windows[window_id].drop_files_callback.call(files); } diff --git a/platform/macos/display_server_macos.mm b/platform/macos/display_server_macos.mm index cfe925e79b3..6461f508185 100644 --- a/platform/macos/display_server_macos.mm +++ b/platform/macos/display_server_macos.mm @@ -938,7 +938,7 @@ Error DisplayServerMacOS::dialog_show(String p_title, String p_description, Vect button_pressed = int64_t(2 + (ret - NSAlertThirdButtonReturn)); } - if (!p_callback.is_null()) { + if (p_callback.is_valid()) { Variant ret; Callable::CallError ce; const Variant *args[1] = { &button_pressed }; @@ -1018,7 +1018,7 @@ Error DisplayServerMacOS::_file_dialog_with_options_show(const String &p_title, String url; url.parse_utf8([[[panel URL] path] UTF8String]); files.push_back(url); - if (!callback.is_null()) { + if (callback.is_valid()) { if (p_options_in_cb) { Variant v_result = true; Variant v_files = files; @@ -1047,7 +1047,7 @@ Error DisplayServerMacOS::_file_dialog_with_options_show(const String &p_title, } } } else { - if (!callback.is_null()) { + if (callback.is_valid()) { if (p_options_in_cb) { Variant v_result = false; Variant v_files = Vector(); @@ -1134,7 +1134,7 @@ Error DisplayServerMacOS::_file_dialog_with_options_show(const String &p_title, url.parse_utf8([[[urls objectAtIndex:i] path] UTF8String]); files.push_back(url); } - if (!callback.is_null()) { + if (callback.is_valid()) { if (p_options_in_cb) { Variant v_result = true; Variant v_files = files; @@ -1163,7 +1163,7 @@ Error DisplayServerMacOS::_file_dialog_with_options_show(const String &p_title, } } } else { - if (!callback.is_null()) { + if (callback.is_valid()) { if (p_options_in_cb) { Variant v_result = false; Variant v_files = Vector(); @@ -1222,7 +1222,7 @@ Error DisplayServerMacOS::dialog_input_text(String p_title, String p_description String ret; ret.parse_utf8([[input stringValue] UTF8String]); - if (!p_callback.is_null()) { + if (p_callback.is_valid()) { Variant v_result = ret; Variant ret; Callable::CallError ce; diff --git a/platform/macos/godot_content_view.mm b/platform/macos/godot_content_view.mm index 93bba84783b..68a7288ad4a 100644 --- a/platform/macos/godot_content_view.mm +++ b/platform/macos/godot_content_view.mm @@ -313,7 +313,7 @@ } DisplayServerMacOS::WindowData &wd = ds->get_window(window_id); - if (!wd.drop_files_callback.is_null()) { + if (wd.drop_files_callback.is_valid()) { Vector files; NSPasteboard *pboard = [sender draggingPasteboard]; diff --git a/platform/macos/godot_window_delegate.mm b/platform/macos/godot_window_delegate.mm index 2d83b460078..7749debfd6d 100644 --- a/platform/macos/godot_window_delegate.mm +++ b/platform/macos/godot_window_delegate.mm @@ -268,7 +268,7 @@ ds->window_resize(window_id, wd.size.width, wd.size.height); - if (!wd.rect_changed_callback.is_null()) { + if (wd.rect_changed_callback.is_valid()) { wd.rect_changed_callback.call(Rect2i(ds->window_get_position(window_id), ds->window_get_size(window_id))); } } @@ -291,7 +291,7 @@ DisplayServerMacOS::WindowData &wd = ds->get_window(window_id); ds->release_pressed_events(); - if (!wd.rect_changed_callback.is_null()) { + if (wd.rect_changed_callback.is_valid()) { wd.rect_changed_callback.call(Rect2i(ds->window_get_position(window_id), ds->window_get_size(window_id))); } } diff --git a/platform/web/display_server_web.cpp b/platform/web/display_server_web.cpp index 06f5eb82f78..a51c161b9c1 100644 --- a/platform/web/display_server_web.cpp +++ b/platform/web/display_server_web.cpp @@ -58,7 +58,7 @@ DisplayServerWeb *DisplayServerWeb::get_singleton() { // Window (canvas) bool DisplayServerWeb::check_size_force_redraw() { bool size_changed = godot_js_display_size_update() != 0; - if (size_changed && !rect_changed_callback.is_null()) { + if (size_changed && rect_changed_callback.is_valid()) { Size2i window_size = window_get_size(); Variant size = Rect2i(Point2i(), window_size); // TODO use window_get_position if implemented. rect_changed_callback.call(size); @@ -109,7 +109,7 @@ void DisplayServerWeb::_drop_files_js_callback(const Vector &p_files) { if (!ds) { ERR_FAIL_MSG("Unable to drop files because the DisplayServer is not active"); } - if (ds->drop_files_callback.is_null()) { + if (!ds->drop_files_callback.is_valid()) { return; } ds->drop_files_callback.call(p_files); @@ -129,7 +129,7 @@ void DisplayServerWeb::request_quit_callback() { void DisplayServerWeb::_request_quit_callback() { DisplayServerWeb *ds = get_singleton(); - if (ds && !ds->window_event_callback.is_null()) { + if (ds && ds->window_event_callback.is_valid()) { Variant event = int(DisplayServer::WINDOW_EVENT_CLOSE_REQUEST); ds->window_event_callback.call(event); } @@ -722,7 +722,7 @@ void DisplayServerWeb::vk_input_text_callback(const char *p_text, int p_cursor) void DisplayServerWeb::_vk_input_text_callback(const String &p_text, int p_cursor) { DisplayServerWeb *ds = DisplayServerWeb::get_singleton(); - if (!ds || ds->input_text_callback.is_null()) { + if (!ds || !ds->input_text_callback.is_valid()) { return; } // Call input_text @@ -972,7 +972,7 @@ void DisplayServerWeb::_send_window_event_callback(int p_notification) { if (godot_js_is_ime_focused() && (p_notification == DisplayServer::WINDOW_EVENT_FOCUS_IN || p_notification == DisplayServer::WINDOW_EVENT_FOCUS_OUT)) { return; } - if (!ds->window_event_callback.is_null()) { + if (ds->window_event_callback.is_valid()) { Variant event = int(p_notification); ds->window_event_callback.call(event); } diff --git a/platform/web/javascript_bridge_singleton.cpp b/platform/web/javascript_bridge_singleton.cpp index d72ad8331b3..a2c83d2f2bb 100644 --- a/platform/web/javascript_bridge_singleton.cpp +++ b/platform/web/javascript_bridge_singleton.cpp @@ -248,7 +248,7 @@ Variant JavaScriptObjectImpl::callp(const StringName &p_method, const Variant ** void JavaScriptObjectImpl::callback(void *p_ref, int p_args_id, int p_argc) { const JavaScriptObjectImpl *obj = (JavaScriptObjectImpl *)p_ref; - ERR_FAIL_COND_MSG(obj->_callable.is_null(), "JavaScript callback failed."); + ERR_FAIL_COND_MSG(!obj->_callable.is_valid(), "JavaScript callback failed."); Vector argp; Array arg_arr; diff --git a/platform/windows/display_server_windows.cpp b/platform/windows/display_server_windows.cpp index b6b713687f9..ebae00017b8 100644 --- a/platform/windows/display_server_windows.cpp +++ b/platform/windows/display_server_windows.cpp @@ -545,7 +545,7 @@ Error DisplayServerWindows::_file_dialog_with_options_show(const String &p_title result->Release(); } } - if (!p_callback.is_null()) { + if (p_callback.is_valid()) { if (p_options_in_cb) { Variant v_result = true; Variant v_files = file_names; @@ -574,7 +574,7 @@ Error DisplayServerWindows::_file_dialog_with_options_show(const String &p_title } } } else { - if (!p_callback.is_null()) { + if (p_callback.is_valid()) { if (p_options_in_cb) { Variant v_result = false; Variant v_files = Vector(); @@ -2556,7 +2556,7 @@ Error DisplayServerWindows::dialog_show(String p_title, String p_description, Ve int button_pressed; if (task_dialog_indirect && SUCCEEDED(task_dialog_indirect(&config, &button_pressed, nullptr, nullptr))) { - if (!p_callback.is_null()) { + if (p_callback.is_valid()) { Variant button = button_pressed; const Variant *args[1] = { &button }; Variant ret; @@ -4591,7 +4591,7 @@ LRESULT DisplayServerWindows::WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARA } if (rect_changed) { - if (!window.rect_changed_callback.is_null()) { + if (window.rect_changed_callback.is_valid()) { window.rect_changed_callback.call(Rect2i(window.last_pos.x, window.last_pos.y, window.width, window.height)); } @@ -4803,7 +4803,7 @@ LRESULT DisplayServerWindows::WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARA files.push_back(file); } - if (files.size() && !windows[window_id].drop_files_callback.is_null()) { + if (files.size() && windows[window_id].drop_files_callback.is_valid()) { windows[window_id].drop_files_callback.call(files); } } break; diff --git a/servers/physics_2d/godot_area_2d.h b/servers/physics_2d/godot_area_2d.h index d14ddb6a2ec..e6c3b45d6c3 100644 --- a/servers/physics_2d/godot_area_2d.h +++ b/servers/physics_2d/godot_area_2d.h @@ -101,10 +101,10 @@ class GodotArea2D : public GodotCollisionObject2D { public: void set_monitor_callback(const Callable &p_callback); - _FORCE_INLINE_ bool has_monitor_callback() const { return !monitor_callback.is_null(); } + _FORCE_INLINE_ bool has_monitor_callback() const { return monitor_callback.is_valid(); } void set_area_monitor_callback(const Callable &p_callback); - _FORCE_INLINE_ bool has_area_monitor_callback() const { return !area_monitor_callback.is_null(); } + _FORCE_INLINE_ bool has_area_monitor_callback() const { return area_monitor_callback.is_valid(); } _FORCE_INLINE_ void add_body_to_query(GodotBody2D *p_body, uint32_t p_body_shape, uint32_t p_area_shape); _FORCE_INLINE_ void remove_body_from_query(GodotBody2D *p_body, uint32_t p_body_shape, uint32_t p_area_shape); diff --git a/servers/physics_3d/godot_area_3d.h b/servers/physics_3d/godot_area_3d.h index c3c9e494a43..701dc739177 100644 --- a/servers/physics_3d/godot_area_3d.h +++ b/servers/physics_3d/godot_area_3d.h @@ -107,10 +107,10 @@ class GodotArea3D : public GodotCollisionObject3D { public: void set_monitor_callback(const Callable &p_callback); - _FORCE_INLINE_ bool has_monitor_callback() const { return !monitor_callback.is_null(); } + _FORCE_INLINE_ bool has_monitor_callback() const { return monitor_callback.is_valid(); } void set_area_monitor_callback(const Callable &p_callback); - _FORCE_INLINE_ bool has_area_monitor_callback() const { return !area_monitor_callback.is_null(); } + _FORCE_INLINE_ bool has_area_monitor_callback() const { return area_monitor_callback.is_valid(); } _FORCE_INLINE_ void add_body_to_query(GodotBody3D *p_body, uint32_t p_body_shape, uint32_t p_area_shape); _FORCE_INLINE_ void remove_body_from_query(GodotBody3D *p_body, uint32_t p_body_shape, uint32_t p_area_shape); diff --git a/servers/rendering/renderer_canvas_cull.cpp b/servers/rendering/renderer_canvas_cull.cpp index 34f9069649c..e48c72cec72 100644 --- a/servers/rendering/renderer_canvas_cull.cpp +++ b/servers/rendering/renderer_canvas_cull.cpp @@ -2106,7 +2106,7 @@ void RendererCanvasCull::update_visibility_notifiers() { if (visibility_notifier->just_visible) { visibility_notifier->just_visible = false; - if (!visibility_notifier->enter_callable.is_null()) { + if (visibility_notifier->enter_callable.is_valid()) { if (RSG::threaded) { visibility_notifier->enter_callable.call_deferred(); } else { @@ -2117,7 +2117,7 @@ void RendererCanvasCull::update_visibility_notifiers() { if (visibility_notifier->visible_in_frame != RSG::rasterizer->get_frame_number()) { visibility_notifier_list.remove(E); - if (!visibility_notifier->exit_callable.is_null()) { + if (visibility_notifier->exit_callable.is_valid()) { if (RSG::threaded) { visibility_notifier->exit_callable.call_deferred(); } else { diff --git a/servers/rendering/renderer_rd/storage_rd/utilities.cpp b/servers/rendering/renderer_rd/storage_rd/utilities.cpp index 8b780a6f7b1..8ff1d2bc468 100644 --- a/servers/rendering/renderer_rd/storage_rd/utilities.cpp +++ b/servers/rendering/renderer_rd/storage_rd/utilities.cpp @@ -198,7 +198,7 @@ void Utilities::visibility_notifier_call(RID p_notifier, bool p_enter, bool p_de ERR_FAIL_NULL(vn); if (p_enter) { - if (!vn->enter_callback.is_null()) { + if (vn->enter_callback.is_valid()) { if (p_deferred) { vn->enter_callback.call_deferred(); } else { @@ -206,7 +206,7 @@ void Utilities::visibility_notifier_call(RID p_notifier, bool p_enter, bool p_de } } } else { - if (!vn->exit_callback.is_null()) { + if (vn->exit_callback.is_valid()) { if (p_deferred) { vn->exit_callback.call_deferred(); } else { diff --git a/tests/display_server_mock.h b/tests/display_server_mock.h index 8d8a678e200..ee7433fcbd4 100644 --- a/tests/display_server_mock.h +++ b/tests/display_server_mock.h @@ -86,7 +86,7 @@ private: } void _send_window_event(WindowEvent p_event) { - if (!event_callback.is_null()) { + if (event_callback.is_valid()) { Variant event = int(p_event); event_callback.call(event); }