Total mess in Gadget_List_Box_Set_List_Length

[xezon]

void Gadget_List_Box_Set_List_Length(GameWindow *list_box, int new_length)
{
    _ListboxData *data = static_cast<_ListboxData *>(list_box->Win_Get_User_Data());
    captainslog_dbgassert(data != nullptr, "We don't have our needed listboxData!");

    if (data != nullptr) {
        captainslog_dbgassert(data->m_columns > 0, "We need at least one Column in the listbox");

        if (data->m_columns >= 1) {
            int columns = data->m_columns;
            _ListEntryRow *row = new _ListEntryRow[new_length];
            captainslog_dbgassert(row != nullptr, "Unable to allocate new data structures for the Listbox");

            if (row != nullptr) {
                memset(row, 0, sizeof(_ListEntryRow) * new_length);

                if (new_length < data->m_listLength) {
                    if (data->m_displayPos > new_length) {
                        data->m_displayPos = new_length;
                    }

                    if (data->m_selectPos > new_length || data->m_multiSelect) {
                        data->m_selectPos = -1;
                    }

                    if (data->m_insertPos > new_length) {
                        data->m_insertPos = new_length;
                    }

                    data->m_endPos = new_length;
                    memcpy(row, data->m_listData, sizeof(_ListEntryRow) * new_length);
                } else {
                    memcpy(row, data->m_listData, sizeof(_ListEntryRow) * data->m_listLength);
                }

                for (int i = 0; i < data->m_listLength; i++) {
                    _ListEntryCell *cell = data->m_listData[i].m_cell;
                    for (int j = columns - 1; j >= 0 && cell != nullptr; --j) {
                        if (i >= new_length && cell[j].m_cellType == LISTBOX_TEXT && i >= new_length) {
                            if (cell[j].m_data != nullptr) {
                                g_theDisplayStringManager->Free_Display_String(static_cast<DisplayString *>(cell[j].m_data));
                            }
                        }
                    }

                    if (i >= new_length) {
                        delete[] data->m_listData[i].m_cell;
                    }

                    data->m_listData[i].m_cell = nullptr;
                }

                data->m_listLength = new_length;

                if (data->m_listData != nullptr) {
                    delete[] data->m_listData;
                }

                data->m_listData = row;

                Compute_Total_Height(list_box);

                if (data->m_listData != nullptr) {
                    if (data->m_multiSelect) {
                        Gadget_List_Box_Remove_Multi_Select(list_box);
                        Gadget_List_Box_Add_Multi_Select(list_box);
                    }
                } else {
                    captainslog_debug("Unable to allocate listbox data pointer");
                }
            }
        }
    }
}

1

First, g_theDisplayStringManager->Free_Display_String(static_cast<DisplayString *>(cell[j].m_data)); will leave a dangling pointer, but only temporarily because it will follow up with delete[] data->m_listData[i].m_cell; further down, so it is ok in principle.

2

Second, if (i >= new_length && cell[j].m_cellType == LISTBOX_TEXT && i >= new_length) tests i >= new_length twice. This test could actually be moved higher up, not deep inside the loop, because it is a waste to test it multiple times. Same for testing cell != nullptr. Is redundant test in loop.

3

                    if (i >= new_length) {
                        delete[] data->m_listData[i].m_cell;
                    }

                    data->m_listData[i].m_cell = nullptr;

Third, this makes no sense. It only deletes when i >= new_length, but sets it to null without condition, so it looks like this can leak memory.

4

if (data->m_listData != nullptr) near the bottom of the function is obsolete test. It already tested row before.

Fixed code, hopefully:

void Gadget_List_Box_Set_List_Length(GameWindow *list_box, int new_length)
{
    _ListboxData *data = static_cast<_ListboxData *>(list_box->Win_Get_User_Data());
    captainslog_dbgassert(data != nullptr, "We don't have our needed listboxData!");

    if (data != nullptr) {
        captainslog_dbgassert(data->m_columns > 0, "We need at least one Column in the listbox");

        if (data->m_columns >= 1) {
            int columns = data->m_columns;
            _ListEntryRow *row = new _ListEntryRow[new_length];
            captainslog_dbgassert(row != nullptr, "Unable to allocate new data structures for the Listbox");

            if (row != nullptr) {
                memset(row, 0, sizeof(_ListEntryRow) * new_length);

                if (new_length < data->m_listLength) {
                    if (data->m_displayPos > new_length) {
                        data->m_displayPos = new_length;
                    }

                    if (data->m_selectPos > new_length || data->m_multiSelect) {
                        data->m_selectPos = -1;
                    }

                    if (data->m_insertPos > new_length) {
                        data->m_insertPos = new_length;
                    }

                    data->m_endPos = new_length;
                    memcpy(row, data->m_listData, sizeof(_ListEntryRow) * new_length);
                } else {
                    memcpy(row, data->m_listData, sizeof(_ListEntryRow) * data->m_listLength);
                }

                for (int i = new_length; i < data->m_listLength; i++) {
                    _ListEntryCell *cell = data->m_listData[i].m_cell;
                    if (cell != nullptr) {
                        for (int j = columns - 1; j >= 0; --j) {
                            if (cell[j].m_cellType == LISTBOX_TEXT && cell[j].m_data != nullptr) {
                                g_theDisplayStringManager->Free_Display_String(
                                    static_cast<DisplayString *>(cell[j].m_data));
                                cell[j].m_data = nullptr;
                            }
                        }

                        delete[] data->m_listData[i].m_cell;
                        data->m_listData[i].m_cell = nullptr;
                    }
                }

                if (data->m_listData != nullptr) {
                    delete[] data->m_listData;
                }

                data->m_listLength = new_length;
                data->m_listData = row;

                Compute_Total_Height(list_box);

                if (data->m_multiSelect) {
                    Gadget_List_Box_Remove_Multi_Select(list_box);
                    Gadget_List_Box_Add_Multi_Select(list_box);
                }
            }
        }
    }
}