Fix #124165: Python api renaming crash due to uninitialized member #124229

Pratik Borhade · 2024-07-05T13:49:19+02:00

Pratik Borhade commented

2024-07-05 13:49:19 +02:00

asset_params is unintialized until asset browser editor is opened.
In case of this crash, renaming update tries to access params->active_file
but param is nullptr. To avoid the crash use ED_fileselect_ensure_active_params,
it will allocate memory for asset_params if null.

`asset_params` is unintialized until asset browser editor is opened. In case of this crash, renaming update tries to access `params->active_file` but param is nullptr. To avoid the crash use `ED_fileselect_ensure_active_params`, it will allocate memory for asset_params if null.

Pratik Borhade added 1 commit 2024-07-05 13:49:31 +02:00

Fix #124165 : Python api renaming crash due to uninitialized member af1a447b11

`asset_params` is unintialized until asset browser editor is opened.
In case of this crash, renaming update tries to access `params->active_file`
but param is nullptr. To avoid the crash use `ED_fileselect_ensure_active_params`,
it will allocate memory for asset_params if null.

Pratik Borhade requested review from Julian Eisel 2024-07-05 13:49:49 +02:00

Pratik Borhade added the

Module

User Interface

label 2024-07-05 13:49:57 +02:00

Pratik Borhade commented

2024-07-05 13:51:40 +02:00

use of const_cast seems illegal, instead we could remove const from function header :)

use of `const_cast` seems illegal, instead we could remove `const` from function header :)

Harley Acheson commented

2024-07-05 21:09:50 +02:00

use of const_cast seems illegal, instead we could remove const from function header :)

Sorry that I have not tested the following, but I think you might be able to move this to the calling site:

diff --git a/source/blender/editors/space_file/space_file.cc b/source/blender/editors/space_file/space_file.cc
index 4f9ee061b8c..91ae506ca0b 100644
--- a/source/blender/editors/space_file/space_file.cc
+++ b/source/blender/editors/space_file/space_file.cc
@@ -393,10 +393,10 @@ static void file_listener(const wmSpaceTypeListenerParams *listener_params)
     case NC_ID: {
       switch (wmn->action) {
         case NA_RENAME: {
+          FileSelectParams *params = ED_fileselect_ensure_active_params(sfile);
           const ID *active_file_id = ED_fileselect_active_asset_get(sfile);
           /* If a renamed ID is active in the file browser, update scrolling to keep it in view. */
           if (active_file_id && (wmn->reference == active_file_id)) {
-            FileSelectParams *params = ED_fileselect_get_active_params(sfile);
             params->rename_id = active_file_id;
             file_params_invoke_rename_postscroll(
                 static_cast<wmWindowManager *>(G_MAIN->wm.first), listener_params->window, sfile);

> use of `const_cast` seems illegal, instead we could remove `const` from function header :) Sorry that I have **not tested** the following, but I _think_ you might be able to move this to the calling site: ```Diff diff --git a/source/blender/editors/space_file/space_file.cc b/source/blender/editors/space_file/space_file.cc index 4f9ee061b8c..91ae506ca0b 100644 --- a/source/blender/editors/space_file/space_file.cc +++ b/source/blender/editors/space_file/space_file.cc @@ -393,10 +393,10 @@ static void file_listener(const wmSpaceTypeListenerParams *listener_params) case NC_ID: { switch (wmn->action) { case NA_RENAME: { + FileSelectParams *params = ED_fileselect_ensure_active_params(sfile); const ID *active_file_id = ED_fileselect_active_asset_get(sfile); /* If a renamed ID is active in the file browser, update scrolling to keep it in view. */ if (active_file_id && (wmn->reference == active_file_id)) { - FileSelectParams *params = ED_fileselect_get_active_params(sfile); params->rename_id = active_file_id; file_params_invoke_rename_postscroll( static_cast<wmWindowManager *>(G_MAIN->wm.first), listener_params->window, sfile); ```

Julian Eisel commented

2024-07-09 17:52:53 +02:00

I can't recreate this with main, I think it's fixed with blender/blender@4b168dd645?

But generally, we shouldn't have to create the "params" here, that should only be done in specific places as part of initialization. This is another way to deal with this.

I can't recreate this with `main`, I think it's fixed with blender/blender@4b168dd6453ea? But generally, we shouldn't have to create the "params" here, that should only be done in specific places as part of initialization. [This](https://pablovazquez.art/paste/#XQAAAQB7AwAAAAAAAAAyGkkJwvwRfVDeCk9H2SRoLFgzPWJpxeINRgxFqCtNibsN+okSPLL9RegoUMZLhSeIaiJhw5msk6wms80L1MgM7eDA94A8tyFSlRBAzGvRhVsHf536Oie6twSigeQYgBOFQsJxekmG7Y1+rXsHe0QXgv5rEVIlZKkGA0Ua6PYpzuewLMTZyNUvO4slcpDJw58g1OYKmIO7OhVw97Zujwu6mbXJLqp/YJ6Ew9jwvPcRIDnHcNStdL9p9GY8KApiyFPp+PgvBofd4B7uNmpe3VeBBxd6TgycRUZ8jtVvXCONwnr3hSBUuY0RkFZY/Au3OtciRzAs/eGRicyBl9gGYWMdINxT4Wts+WuX7Bn3YCo/lUdKEclkiwSCsuheqso2WXTi8RLWAUM5l7v7M8ntP3x1bk4qIM/8bm/uRxzjKRKXMw0V7XBYjsfYF/LZCL7ZyZQLd1vOOANtA2EVG/oSR/xR94NZBT/juFqUqp27xFUApGrqMCl1iF8b2enDdFkXSrPMY/M04sNr+dxXEXYQc7pbb5X+CcTTvIcgErFqjGwAWSeuYtm9lfb44cGh+2FIeUfTVugAHd9Wv6Nx15fIINJzOHpoOs/gM7loTDVdjlkz/wWQHAA=) is another way to deal with this.

Harley Acheson commented

2024-07-09 18:29:17 +02:00

I can't recreate this with main, I think it's fixed with blender/blender@4b168dd645?

It is!

@PratikPB2123 - It looks like this no longer needed

> I can't recreate this with `main`, I think it's fixed with blender/blender@4b168dd6453ea? It is! @PratikPB2123 - It looks like this no longer needed

Pratik Borhade commented

2024-07-10 12:05:21 +02:00

Indeed :)

Pratik Borhade closed this pull request

2024-07-10 12:05:24 +02:00

Pratik Borhade deleted branch 124165-crash-rename-update

2024-07-10 12:05:41 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #124165: Python api renaming crash due to uninitialized member #124229

Pull request closed