griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

BKE_idprop: string size argument consistency, fix off by one errors

Browse Source 
The length passed to IDP_NewString included the nil terminator
unlike IDP_AssignString. Callers to IDP_AssignString from MOD_nodes.cc
passed in an invalid size for e.g. There where also callers passing in
the strlen(..) of the value unnecessarily.

Resolve with the following changes:

- Add `*MaxSize()` versions of IDP_AssignString & IDP_NewString which
  take a size argument.

- Remove the size argument from the existing functions as most callers
  don't need to clamp the length of the string, avoid calculating &
  passing in length values when they're unnecessary.

- When clamping the size is needed, both functions now include the nil
  byte in the size since this is the convention for BLI_string and other
  BLI API's dealing with nil terminated strings,
  it avoids having to pass in `sizeof(value) - 1`.
This commit is contained in:
Campbell Barton
2023-05-02 15:07:55 +10:00
parent d5d3305280
commit 2483c19ad3
8 changed files with 55 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
source/blender/blenkernel/BKE_idprop.h
View File

@@ -72,15 +72,20 @@ void IDP_FreeArray(struct IDProperty *prop);
/**
* \param st: The string to assign.
* \param name: The property name.
* \param maxlen: The size of the new string (including the \0 terminator).
* \param maxncpy: The maximum size of the string (including the `\0` terminator).
* \return The new string property.
*/
struct IDProperty *IDP_NewString(const char *st,
const char *name,
int maxlen) ATTR_WARN_UNUSED_RESULT
ATTR_NONNULL(2 /* 'name 'arg */); /* maxlen excludes '\0' */
void IDP_AssignString(struct IDProperty *prop, const char *st, int maxlen)
ATTR_NONNULL(); /* maxlen excludes '\0' */
struct IDProperty *IDP_NewStringMaxSize(const char *st,
const char *name,
int maxncpy) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(2);
struct IDProperty *IDP_NewString(const char *st, const char *name) ATTR_WARN_UNUSED_RESULT
ATTR_NONNULL(2);
/**
* \param st: The string to assign.
* \param maxncpy: The maximum size of the string (including the `\0` terminator).
*/
void IDP_AssignStringMaxSize(struct IDProperty *prop, const char *st, int maxncpy) ATTR_NONNULL();
void IDP_AssignString(struct IDProperty *prop, const char *st) ATTR_NONNULL();
void IDP_ConcatStringC(struct IDProperty *prop, const char *st) ATTR_NONNULL();
void IDP_ConcatString(struct IDProperty *str1, struct IDProperty *append) ATTR_NONNULL();
void IDP_FreeString(struct IDProperty *prop) ATTR_NONNULL();

43
source/blender/blenkernel/intern/idprop.c
View File

@@ -342,7 +342,7 @@ static IDProperty *IDP_CopyArray(const IDProperty *prop, const int flag)
/** \name String Functions (IDProperty String API)
* \{ */
IDProperty *IDP_NewString(const char *st, const char *name, int maxlen)
IDProperty *IDP_NewStringMaxSize(const char *st, const char *name, int maxncpy)
{
IDProperty *prop = MEM_callocN(sizeof(IDProperty), "IDProperty string");
@@ -356,13 +356,16 @@ IDProperty *IDP_NewString(const char *st, const char *name, int maxlen)
/* include null terminator '\0' */
int stlen = (int)strlen(st) + 1;
if (maxlen > 0 && maxlen < stlen) {
stlen = maxlen;
if ((maxncpy > 0) && (maxncpy < stlen)) {
stlen = maxncpy;
}
prop->data.pointer = MEM_mallocN((size_t)stlen, "id property string 2");
prop->len = prop->totallen = stlen;
BLI_strncpy(prop->data.pointer, st, (size_t)stlen);
if (stlen > 0) {
memcpy(prop->data.pointer, st, (size_t)stlen);
IDP_String(prop)[stlen - 1] = '\0';
}
}
prop->type = IDP_STRING;
@@ -371,6 +374,11 @@ IDProperty *IDP_NewString(const char *st, const char *name, int maxlen)
return prop;
}
IDProperty *IDP_NewString(const char *st, const char *name)
{
return IDP_NewStringMaxSize(st, name, -1);
}
static IDProperty *IDP_CopyString(const IDProperty *prop, const int flag)
{
BLI_assert(prop->type == IDP_STRING);
@@ -386,23 +394,26 @@ static IDProperty *IDP_CopyString(const IDProperty *prop, const int flag)
return newp;
}
void IDP_AssignString(IDProperty *prop, const char *st, int maxlen)
void IDP_AssignStringMaxSize(IDProperty *prop, const char *st, int maxncpy)
{
BLI_assert(prop->type == IDP_STRING);
int stlen = (int)strlen(st);
if (maxlen > 0 && maxlen < stlen) {
stlen = maxlen;
const bool is_byte = prop->subtype == IDP_STRING_SUB_BYTE;
int stlen = (int)strlen(st) + (is_byte ? 0 : 1);
if ((maxncpy > 0) && (maxncpy < stlen)) {
stlen = maxncpy;
}
if (prop->subtype == IDP_STRING_SUB_BYTE) {
IDP_ResizeArray(prop, stlen);
IDP_ResizeArray(prop, stlen);
if (stlen > 0) {
memcpy(prop->data.pointer, st, (size_t)stlen);
if (is_byte == false) {
IDP_String(prop)[stlen - 1] = '\0';
}
}
else {
stlen++;
IDP_ResizeArray(prop, stlen);
BLI_strncpy(prop->data.pointer, st, (size_t)stlen);
}
}
void IDP_AssignString(IDProperty *prop, const char *st)
{
IDP_AssignStringMaxSize(prop, st, 0);
}
void IDP_ConcatStringC(IDProperty *prop, const char *st)

2
source/blender/blenkernel/intern/idprop_create.cc
View File

@@ -49,7 +49,7 @@ std::unique_ptr<IDProperty, IDPropertyDeleter> create(const StringRefNull prop_n
std::unique_ptr<IDProperty, IDPropertyDeleter> create(const StringRefNull prop_name,
const StringRefNull value)
{
IDProperty *property = IDP_NewString(value.c_str(), prop_name.c_str(), value.size() + 1);
IDProperty *property = IDP_NewString(value.c_str(), prop_name.c_str());
return std::unique_ptr<IDProperty, IDPropertyDeleter>(property);
}

8
source/blender/editors/interface/interface.cc
View File

@@ -1275,7 +1275,7 @@ static bool ui_but_event_operator_string_from_menu(const bContext *C,
/* annoying, create a property */
const IDPropertyTemplate val = {0};
IDProperty *prop_menu = IDP_New(IDP_GROUP, &val, __func__); /* Dummy, name is unimportant. */
IDP_AddToGroup(prop_menu, IDP_NewString(mt->idname, "name", sizeof(mt->idname)));
IDP_AddToGroup(prop_menu, IDP_NewStringMaxSize(mt->idname, "name", sizeof(mt->idname)));
if (WM_key_event_operator_string(
C, "WM_OT_call_menu", WM_OP_INVOKE_REGION_WIN, prop_menu, true, buf, buf_len))
@@ -1302,7 +1302,7 @@ static bool ui_but_event_operator_string_from_panel(const bContext *C,
const IDPropertyTemplate group_val = {0};
IDProperty *prop_panel = IDP_New(
IDP_GROUP, &group_val, __func__); /* Dummy, name is unimportant. */
IDP_AddToGroup(prop_panel, IDP_NewString(pt->idname, "name", sizeof(pt->idname)));
IDP_AddToGroup(prop_panel, IDP_NewStringMaxSize(pt->idname, "name", sizeof(pt->idname)));
IDPropertyTemplate space_type_val = {0};
space_type_val.i = pt->space_type;
IDP_AddToGroup(prop_panel, IDP_New(IDP_INT, &space_type_val, "space_type"));
@@ -1490,7 +1490,7 @@ static bool ui_but_event_property_operator_string(const bContext *C,
const IDPropertyTemplate group_val = {0};
prop_path = IDP_New(IDP_GROUP, &group_val, __func__);
if (data_path) {
IDP_AddToGroup(prop_path, IDP_NewString(data_path, "data_path", strlen(data_path) + 1));
IDP_AddToGroup(prop_path, IDP_NewString(data_path, "data_path"));
}
if (prop_enum_value_ok) {
const EnumPropertyItem *item;
@@ -1507,7 +1507,7 @@ static bool ui_but_event_property_operator_string(const bContext *C,
}
else {
const char *id = item[index].identifier;
prop_value = IDP_NewString(id, prop_enum_value_id, strlen(id) + 1);
prop_value = IDP_NewString(id, prop_enum_value_id);
}
IDP_AddToGroup(prop_path, prop_value);
}

2
source/blender/editors/interface/interface_context_menu.cc
View File

@@ -66,7 +66,7 @@ static IDProperty *shortcut_property_from_rna(bContext *C, uiBut *but)
/* Create ID property of data path, to pass to the operator. */
const IDPropertyTemplate val = {0};
IDProperty *prop = IDP_New(IDP_GROUP, &val, __func__);
IDP_AddToGroup(prop, IDP_NewString(final_data_path, "data_path", strlen(final_data_path) + 1));
IDP_AddToGroup(prop, IDP_NewString(final_data_path, "data_path"));
MEM_freeN((void *)final_data_path);

4
source/blender/imbuf/intern/metadata.c
View File

@@ -81,10 +81,10 @@ void IMB_metadata_set_field(struct IDProperty *metadata, const char *key, const
}
if (prop) {
IDP_AssignString(prop, value, 0);
IDP_AssignString(prop, value);
}
else if (prop == NULL) {
prop = IDP_NewString(value, key, 0);
prop = IDP_NewString(value, key);
IDP_AddToGroup(metadata, prop);
}
}

7
source/blender/makesrna/intern/rna_access.cc
View File

@@ -3395,7 +3395,7 @@ void RNA_property_string_set(PointerRNA *ptr, PropertyRNA *prop, const char *val
if ((idprop = rna_idproperty_check(&prop, ptr))) {
/* both IDP_STRING_SUB_BYTE / IDP_STRING_SUB_UTF8 */
IDP_AssignString(idprop, value, RNA_property_string_maxlength(prop) - 1);
IDP_AssignStringMaxSize(idprop, value, RNA_property_string_maxlength(prop));
rna_idproperty_touch(idprop);
}
else if (sprop->set) {
@@ -3409,8 +3409,9 @@ void RNA_property_string_set(PointerRNA *ptr, PropertyRNA *prop, const char *val
group = RNA_struct_idprops(ptr, 1);
if (group) {
IDP_AddToGroup(group,
IDP_NewString(value, prop->identifier, RNA_property_string_maxlength(prop)));
IDP_AddToGroup(
group,
IDP_NewStringMaxSize(value, prop->identifier, RNA_property_string_maxlength(prop)));
}
}
}

8
source/blender/modifiers/intern/MOD_nodes.cc
View File

@@ -671,7 +671,7 @@ static void update_input_properties_from_node_tree(const bNodeTree &tree,
if (old_properties == nullptr) {
if (socket.default_attribute_name && socket.default_attribute_name[0] != '\0') {
IDP_AssignString(attribute_prop, socket.default_attribute_name, MAX_NAME);
IDP_AssignStringMaxSize(attribute_prop, socket.default_attribute_name, MAX_NAME);
IDP_Int(use_attribute_prop) = 1;
}
}
@@ -705,7 +705,7 @@ static void update_output_properties_from_node_tree(const bNodeTree &tree,
}
const std::string idprop_name = socket.identifier + attribute_name_suffix;
IDProperty *new_prop = IDP_NewString("", idprop_name.c_str(), MAX_NAME);
IDProperty *new_prop = IDP_NewString("", idprop_name.c_str());
if (socket.description[0] != '\0') {
IDPropertyUIData *ui_data = IDP_ui_data_ensure(new_prop);
ui_data->description = BLI_strdup(socket.description);
@@ -714,7 +714,7 @@ static void update_output_properties_from_node_tree(const bNodeTree &tree,
if (old_properties == nullptr) {
if (socket.default_attribute_name && socket.default_attribute_name[0] != '\0') {
IDP_AssignString(new_prop, socket.default_attribute_name, MAX_NAME);
IDP_AssignStringMaxSize(new_prop, socket.default_attribute_name, MAX_NAME);
}
}
else {
@@ -1498,7 +1498,7 @@ static void attribute_search_exec_fn(bContext *C, void *data_v, void *item_v)
const std::string attribute_prop_name = data.socket_identifier + attribute_name_suffix;
IDProperty &name_property = *IDP_GetPropertyFromGroup(nmd->settings.properties,
attribute_prop_name.c_str());
IDP_AssignString(&name_property, item.name.c_str(), 0);
IDP_AssignString(&name_property, item.name.c_str());
ED_undo_push(C, "Assign Attribute Name");
}