Adding and Deleting elements on a Array of Structs Using memcpy()

Question

So I have this Contact struct and an array that holds a bunch of instances of Contact. My problem is that I use memcpy and have tried using mmove for this as well to "delete" and "add" contact elements to this array. It seems to work perfectly fine when I debug and run through the program. I can track my contacts being added and removed from the array but when I run the program without debug and without stepping through the program crashes!

This is my Contact struct:

typedef struct contact Contact;
typedef struct contact *pContact;
struct contact {
    char lastName[BUFSIZ];
    char firstName[BUFSIZ];
    char email[BUFSIZ];
    char pNumber[BUFSIZ];
};

This is how a contact is created:

struct Contact *contactArr[1024];
int size = 0;
Contact* CreateContact(int pos, char *info) {
    Contact *pContactNewContact = (Contact*) malloc(sizeof(Contact));
    char *lastName = strtok(info, ",");
    char *firstName = strtok(NULL, ",");
    char *email = strtok(NULL, ",");
    char *pNumber = strtok(NULL, ",");
    if (pContactNewContact) {
        strcpy(pContactNewContact->lastName, lastName);
        strcpy(pContactNewContact->firstName, firstName);
        strcpy(pContactNewContact->email, email);
        strcpy(pContactNewContact->pNumber, pNumber);
    }
    InsertContact(pos, pContactNewContact);
    return pContactNewContact;
}

These are my array manipulating functions.

void InsertContact(int pos, pContact *insert) {
    if (size == 0)
        contactArr[0] = insert;
    else {
        memmove((contactArr + pos + 1), (contactArr + pos),
                (size + 1) * sizeof(Contact));
        contactArr[pos] = insert;
    }
    size++;
}

void DelContact(int pos) {
    if (pos == 0) {
        memmove(contactArr, (contactArr + 1), (size - 1) * sizeof(Contact));
        contactArr[pos] = 0;
    } else if (pos <= size) {
        memmove((contactArr + pos - 1), (contactArr + pos),
                (size - pos) * sizeof(Contact));
        contactArr[pos] = 0;
    }
    size--;
}

Code is too incomplete. Please provide a [complete minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example). But certainly `memcpy` should not be used for overlapping src and dest. Show the `memove` version instead when you update the post with the complete minimal code. — kaylum, Apr 29 '21 at 01:34
You can't use `memcpy()` if the source and destination overlap. You have to use `memmove()` — Barmar, Apr 29 '21 at 01:37
Right that is what I gathered from similar posts, although even when using memmove() there was the same problem. I appreciate the input though I will do away with the memcpy(). — Jonnes, Apr 29 '21 at 01:40
I think you have some off-by-one errors in your calculations. It would be easier if you wrote `&contactArr[pos]` instead of using pointer arithmetic. — Barmar, Apr 29 '21 at 01:41
And why are you assigning 0 after you've shifted the array elements? You're zapping the contact that replaced the one that was deleted. — Barmar, Apr 29 '21 at 01:42
When you're deleting a contact, you should free it before you shift the remaining elements, otherwise you have lots of memory leaks. — Barmar, Apr 29 '21 at 01:44
so in DelContact I should free(contactArr[pos]) before memmove? also I assign the contactArr[pos] = 0 so that there can be new information stored there? I could be misunderstanding something I am pretty new to C. Thanks for all the input! — Jonnes, Apr 29 '21 at 01:48

Barmar · Accepted Answer · 2021-04-29T03:54:34.983

All your sizeof(Contact) multiplications should be sizeof(pContact), since it's an array of pointers, not an array of contacts.

The amount you're moving should be size - pos, not size + 1.

You need to free the contact before overwriting its position in DelContact. You don't need to assign a null pointer to pos, but you can assign it to the extra element at the end.

It's not necessary to special-case pos == 0 when deleting. All the generic calculations will work correctly using that value of pos.

You need to subtract 1 from size - pos when deleting.

pos <= size should be pos < size.

InsertContact should also have a check that you haven't reached the capacity of the array. I've called that maxSize below.

You shouldn't increment or decrement size if the if checks failed, since nothing was actually changed.

void InsertContact(int pos, pContact *insert) {
    if (size == 0) {
        contactArr[0] = insert;
        size = 1;
    } else if (size <= maxSize && pos <= size) {
        memmove(&contactArr[pos+1], &contactArr[pos],
                (size - pos) * sizeof(pContact));
        contactArr[pos] = insert;
        size++;
    }
}

void DelContact(int pos) {
    if (pos < size) {
        free(contactArr[pos]);
        memmove(&contactArr[pos], &contactArr[pos+1],
                (size - pos - 1) * sizeof(pContact));
        size--;
        contactArr[size] = 0;
    }
}

Thank you so much for the help. All of these corrections make sense to me except for when pos = size and we are inserting. Should there be a special case for that? — Jonnes, Apr 29 '21 at 02:34
You don't need a special case for that. `(size-pos)` will be `0`, so `memmove()` won't do anything. — Barmar, Apr 29 '21 at 02:35
But if that is the case that pos = size then else if (size <= maxSize && pos < size) will prevent the memmove() from executing. pos is an index not element position. — Jonnes, Apr 29 '21 at 03:05
I had that test wrong, it should be `size <= maxSize && pos <= size` — Barmar, Apr 29 '21 at 03:54

Adding and Deleting elements on a Array of Structs Using memcpy()

1 Answers1