Skip to content

Commit

Permalink
Fix some issues in getitemgroupitems
Browse files Browse the repository at this point in the history
- Memory leak in the deduplication function
- Some unnecessary data copies in the search and deduplication function
- Accidental overwrite of the original group's data in the deduplication
  function

Signed-off-by: Haru <[email protected]>
  • Loading branch information
MishimaHaruna committed Apr 30, 2024
1 parent 7cbc70c commit b665c4c
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 80 deletions.
24 changes: 10 additions & 14 deletions src/map/itemdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -348,23 +348,19 @@ static bool itemdb_in_group(struct item_group *group, int nameid)
}

/**
* Search for a group of item
* @param group return information about group
* Search for an item group.
*
* @param nameid item group id to search
* @return bool true if found, false otherwise
* @return A pointer to the group
* @retval NULL if the group was not found
*/
static bool itemdb_search_group(struct item_group *group, int nameid)
static const struct item_group *itemdb_search_group(int nameid)
{
for (int i = 0; i < itemdb->group_count; i++) {
struct item_group g = itemdb->groups[i];

if (g.id == nameid) {
*group = g;
return true;
}
}

return false;
int i;
ARR_FIND(0, itemdb->group_count, i, itemdb->groups[i].id == nameid);
if (i != itemdb->group_count)
return &itemdb->groups[i];
return NULL;
}

/// Searches for the item_data.
Expand Down
2 changes: 1 addition & 1 deletion src/map/itemdb.h
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ struct itemdb_interface {
struct itemdb_option* (*option_exists) (int idx);
struct item_reform* (*reform_exists) (int idx);
bool (*in_group) (struct item_group *group, int nameid);
bool (*search_group) (struct item_group *group, int nameid);
const struct item_group *(*search_group) (int nameid);
int (*group_item) (struct item_group *group);
int (*chain_item) (unsigned short chain_id, int *rate);
void (*package_item) (struct map_session_data *sd, struct item_package *package);
Expand Down
90 changes: 25 additions & 65 deletions src/map/script.c
Original file line number Diff line number Diff line change
Expand Up @@ -9133,46 +9133,6 @@ static BUILDIN(grouprandomitem)
return true;
}

/*==========================================
* utils for item group
*------------------------------------------*/
static bool utils_itemgroup_remove_duplicate(struct item_group *group)
{
int n = group->qty;
int *arr = group->nameid;
int i, j, k;

// Remove duplicates
for (i = 0; i < n; i++) {
for (j = i + 1; j < n; j++) {
if (arr[i] == arr[j]) {
// Shift elements to the left
for (k = j; k < n - 1; k++) {
arr[k] = arr[k + 1];
}
n--;
j--;
}
}
}

// Create a new array without duplicates
int *newArr = malloc(n * sizeof(int));
if (newArr == NULL) {
// Handle memory allocation error
return false;
}

for (i = 0; i < n; i++) {
newArr[i] = arr[i];
}

group->qty = n;
group->nameid = newArr;

return true;
}

/*==========================================
* getitemgroupitems <reference array of item_id>, <item_group_id>;
*------------------------------------------*/
Expand All @@ -9188,27 +9148,6 @@ static BUILDIN(getitemgroupitems)
return false;// not a variable
}

int nameid = script_getnum(st, 3);
struct item_data *item = itemdb->exists(nameid);
if (item == NULL) {
ShowError("buildin_getitemgroupitems: invalid item %d\n", nameid);
script_pushint(st, 0);
return false;
}

struct item_group group;
if (itemdb->search_group(&group, item->nameid) == false) {
ShowWarning("buildin_getitemgroupitems: item group not found\n");
script_pushint(st, 0);
return false;
}

if (!utils_itemgroup_remove_duplicate(&group)) {
ShowError("buildin_getitemgroupitems: failed to remove duplicates\n");
script_pushint(st, 0);
return false;
}

int32 idata = reference_getindex(data);
int32 dataid = reference_getid(data);
const char *data_name = reference_getname(data);
Expand All @@ -9228,12 +9167,33 @@ static BUILDIN(getitemgroupitems)
st->state = END;
return false;
}


int nameid = script_getnum(st, 3);
struct item_data *item = itemdb->exists(nameid);
if (item == NULL) {
ShowError("buildin_getitemgroupitems: invalid item %d\n", nameid);
script_pushint(st, 0);
return false;
}

const struct item_group *group = itemdb->search_group(item->nameid);
if (group == NULL) {
ShowWarning("buildin_getitemgroupitems: item group not found\n");
script_pushint(st, 0);
return false;
}

int count = 0;
for (int i = 0; i < group.qty; i++) {
int id = group.nameid[i];
for (int i = 0; i < group->qty; i++) {
int id = group->nameid[i];
int j = 0;
ARR_FIND(0, i, j, group->nameid[i] == group->nameid[j]);
if (i != j) {
// Already encountered - skip duplicates
continue;
}
const void *v = (const void *)h64BPTRSIZE(id);
script->set_reg(st, NULL, reference_uid(dataid, idata + i), data_name, v, reference_getref(data));
script->set_reg(st, NULL, reference_uid(dataid, idata + count), data_name, v, reference_getref(data));
count++;
}

Expand Down

0 comments on commit b665c4c

Please sign in to comment.