Fix: AssetShelfSettings memory management issues (assignment & destructor)
Rewrite `AssetShelfSettings::operator=` and fix `~AssetShelfSettings()` to address some memory management issues: - `memcpy` was used to copy a C++ struct, which shouldn't be done: it can cause undefined behaviour if the class has vtables or internal padding. This isn't the case here, but GCC nonetheless warns for it. - `active_catalog_path` was incorrectly freed via `MEM_delete()`; it was allocated with `BLI_strdup()`, which uses `MEM_malloc_arrayN()` and thus should be freed with `MEM_freeN()`. - Self-assignments weren't handled properly, and could cause data loss. These are now all fixed. Pull Request: https://projects.blender.org/blender/blender/pulls/143701
This commit is contained in:
@@ -17,6 +17,7 @@
|
||||
|
||||
#include "BLI_listbase.h"
|
||||
#include "BLI_string.h"
|
||||
#include "BLI_string_utf8.h"
|
||||
|
||||
#include "BKE_asset.hh"
|
||||
#include "BKE_preferences.h"
|
||||
@@ -36,13 +37,24 @@ AssetShelfSettings::AssetShelfSettings(const AssetShelfSettings &other)
|
||||
|
||||
AssetShelfSettings &AssetShelfSettings::operator=(const AssetShelfSettings &other)
|
||||
{
|
||||
/* Start with a shallow copy. */
|
||||
memcpy(this, &other, sizeof(AssetShelfSettings));
|
||||
|
||||
if (active_catalog_path) {
|
||||
active_catalog_path = BLI_strdup(other.active_catalog_path);
|
||||
if (this == &other) {
|
||||
return *this; /* Handle self-assignment safely. */
|
||||
}
|
||||
enabled_catalog_paths = BKE_asset_catalog_path_list_duplicate(other.enabled_catalog_paths);
|
||||
|
||||
/* Free existing properties. */
|
||||
BKE_asset_catalog_path_list_free(this->enabled_catalog_paths);
|
||||
MEM_SAFE_FREE(this->active_catalog_path);
|
||||
|
||||
/* Copy from 'other'. */
|
||||
this->asset_library_reference = other.asset_library_reference;
|
||||
STRNCPY_UTF8(this->search_string, other.search_string);
|
||||
this->preview_size = other.preview_size;
|
||||
this->display_flag = other.display_flag;
|
||||
|
||||
if (other.active_catalog_path) {
|
||||
this->active_catalog_path = BLI_strdup(other.active_catalog_path);
|
||||
}
|
||||
this->enabled_catalog_paths = BKE_asset_catalog_path_list_duplicate(other.enabled_catalog_paths);
|
||||
|
||||
return *this;
|
||||
}
|
||||
@@ -50,7 +62,7 @@ AssetShelfSettings &AssetShelfSettings::operator=(const AssetShelfSettings &othe
|
||||
AssetShelfSettings::~AssetShelfSettings()
|
||||
{
|
||||
BKE_asset_catalog_path_list_free(enabled_catalog_paths);
|
||||
MEM_delete(active_catalog_path);
|
||||
MEM_SAFE_FREE(active_catalog_path);
|
||||
}
|
||||
|
||||
namespace blender::ed::asset::shelf {
|
||||
|
||||
Reference in New Issue
Block a user