CODEWITHSUNDEEP
carrayspointersstructdynamic-allocation
Drakalex
Drakalex

Reputation: 1538

Remove element from dynamic array of structure

I'm working in C

I have a struct called Entity and I create a dynamic array of that struct. Then I try to remove one element from the array but I don't get the behaviour I want.

Here is the code I'm using:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct Entity
{
    int x, y;
    int velX, velY;
}Entity;

int remove_element(Entity** array, int sizeOfArray, int indexToRemove)
{
    int i;

    printf("Beginning processing. Array is currently: ");
    for (i = 0; i < sizeOfArray; ++i)
        printf("%d ", (*array)[i].x);
    printf("\n");

    Entity* temp = malloc((sizeOfArray - 1) * sizeof(Entity)); // allocate an array with a size 1 less than the current one

    memmove(
            temp,
            *array,
            (indexToRemove+1)*sizeof(Entity)); // copy everything BEFORE the index

    memmove(
            temp+indexToRemove,
            (*array)+(indexToRemove+1),
            (sizeOfArray - indexToRemove)*sizeof(Entity)); // copy everything AFTER the index


    printf("Processing done. Array is currently: ");
    for (i = 0; i < sizeOfArray - 1; ++i)
        printf("%d ", (temp)[i].x);
    printf("\n");

    free (*array);
    *array = temp;
    return 0;

}

int main()
{
    int i;
    int howMany = 20;

    Entity* test = malloc(howMany * sizeof(Entity*));

    for (i = 0; i < howMany; ++i)
        (test[i].x) = i;

    remove_element(&test, howMany, 14);
    --howMany;

    return 0;
}

And the output I get :

Beginning processing. Array is currently: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19
Processing done. Array is currently: 0 1 2 3 4 1866386284 6 7 8 9 10 11 12 13 15 16 17 18 19

Then the program crashes at the free (*array); line. I want my second line to be 0 1 2 3 4 5 6 7 8 9 10 11 12 13 15 16 17 18 19.

How could I solve my problem ?

Upvotes: 2

Views: 701

Answers (4)

user9736874
user9736874

Reputation:

a little touch

remove element in any type of struct array

regards

int remove_element(void **input_ptr, int input_size, int index_remove, int struct_size)
{
    void *temp_ptr;

    temp_ptr = malloc((input_size - 1) * struct_size);
    if (temp_ptr == 0)
        return -1;

    memmove(temp_ptr, *input_ptr, index_remove * struct_size);

    memmove(temp_ptr + (index_remove * struct_size), (*input_ptr) + (index_remove + 1) * struct_size, (input_size - index_remove - 1) * struct_size);

    free(*input_ptr);

    *input_ptr = temp_ptr;

    return 1;
}

usage example for question struct

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct Entity
{
    int x, y;
    int velX, velY;
}Entity;

int remove_element(void **input_ptr, int input_size, int index_remove, int struct_size)
{
    void *temp_ptr;

    temp_ptr = malloc((input_size - 1) * struct_size);
    if (temp_ptr == 0)
        return -1;

    memmove(temp_ptr, *input_ptr, index_remove * struct_size);

    memmove(temp_ptr + (index_remove * struct_size), (*input_ptr) + (index_remove + 1) * struct_size, (input_size - index_remove - 1) * struct_size);

    free(*input_ptr);

    *input_ptr = temp_ptr;

    return 1;
}

int main()
{
    int i;
    int howMany = 20;

    Entity* test = malloc(howMany * sizeof(Entity));

    for (i = 0; i < howMany; ++i)
    {
        (test[i].x) = i;
        printf("test[%d].x = '%d'\n", i, test[i].x);
    }

    remove_element((void**)&test, howMany, 14, sizeof(Entity));
    --howMany;

    printf("Deleted index --- new array\n");
    for (i = 0; i < howMany; ++i)
        printf("test[%d].x = '%d'\n", i, test[i].x);

    return 0;
}

Upvotes: 0

Rohit
Rohit

Reputation: 152

In main itself your memeory allocation is not done properly.if you are using double pointer you should allocate memory first for double pointer and than single pointer in loop one by one.

Upvotes: 0

chqrlie
chqrlie

Reputation: 144695

Your offset calculations are off by one in both memmove instances. Use this instead:

// copy everything BEFORE the index
memmove(temp,
        *array,
        indexToRemove * sizeof(Entity));

// copy everything AFTER the index
memmove(temp + indexToRemove,
        *array + indexToRemove + 1,
        (sizeOfArray - indexToRemove - 1) * sizeof(Entity));

Upvotes: 0

user2736738
user2736738

Reputation: 30926

First thing you have allocated memory space for holding 20 Enity*. Then you have dereferenced it (and the value it contained is indeterminate). This is undefined behavior. And all story ends here.

But let's analyze what you mostly wanted.

Entity* test = malloc(howMany * sizeof(Entity));
                                       ^^^^^^^

is what you wanted. Because only if you do this you will get the member elements x and so on.

Also if you are considering 0 indexing then the memmove calls should be

memmove(temp, *array, (indexToRemove)*sizeof(Entity)); 
memmove(temp+indexToRemove, (*array)+(indexToRemove+1), 
            (sizeOfArray - indexToRemove - 1)*sizeof(Entity));

These two changes will be enough to solve the problems you are facing and realizing the correct behavior. (If this is all there is in your code).

Also as per standard the main() should be declared like this in case it doesn't take any parameter int main(void). Free the dynamically allocated memory when you are done working with it. Also you should check the return value of malloc - in case it fails it returns NULL and you should handle that case.

Upvotes: 2

Related Questions