2

I have this "array" of pointers to structures initially allocated by a malloc, and I need to add additional memory by an increment when the initial capacity becomes full. When I try to allocate more memory, the first element seems to vanish and causes my program to crash. Can anyone help?

#include "a2.h" #include <stdio.h> #include <stdlib.h> #include <string.h> int idCompare ( const void * a, const void * b ){ message *m1 = *(message **)a; message *m2 = *(message **)b; return m1->messageId - m2->messageId; } int textCompare( const void *a, const void *b ) { message *m1 = *(message **)a; message *m2 = *(message **)b; return strcmp(m1->messageText, m2->messageText); } int main(void) { int id, i; int count = 0; char cmd[MAX_CMD_LEN]; char msg_text[MAX_TEXT_LEN]; message **mList = malloc(INITIAL_CAPACITY * sizeof(message)); int capacity = INITIAL_CAPACITY * sizeof(message); int size = 0; while (scanf("%s", cmd)){ if (!strcmp(cmd, "add")){ printf("(pre) Capacity =%d, Count =%d, Size =%d\n", capacity, count, size); int found = 0; scanf("%d\n", &id); fgets(msg_text, sizeof(msg_text), stdin); if(size >= capacity){ capacity = size; *(mList + count) = malloc(CAPACITY_INCREMENT * sizeof(message)); message *p = *(mList + count); if(p == NULL){ //If malloc fails, free mList and exit. printf("out of memory\n"); free(*mList); exit(1); } } for(i = 0; i < count; i++){ //If existing id is found, send flag & do not add. message *p = *(mList + i); if(p->messageId == id){ found = 1; } } if(!found){ message *p = *(mList + count); p->messageId = id; p->messageText = malloc(strlen(msg_text)+1); strcpy(p->messageText, msg_text); count++; size += 10 * sizeof(message); printf("(post) Capacity =%d, Count =%d, Size =%d\n", capacity, count, size); } } else if (!strcmp(cmd, "delete")){ //Cycle through mList, if id is found, shift elements left. scanf("%d", &id); for(i = 0; i < count; i++){ message *p = *(mList + i); if(p->messageId == id){ for(;i < count; i++){*(mList + i) = *(mList + i + 1);} count--; size -= 10 * sizeof(message); } } } else if (!strcmp(cmd, "find")){ //Cycle through mList, if id is matched, print to stdout. scanf("%d", &id); for(i = 0; i < count; i++){ message *p = *(mList + i); if(p->messageId == id){ printf("%s", p->messageText); } } } else if (!strcmp(cmd, "output")){ //Cycle through mList, print all to stdout. for(i = 0; i < count; i++){ message *p = *(mList + i); printf("%s", p->messageText); } } else if (!strcmp(cmd, "sortById")){ qsort (mList, count, sizeof(message*), idCompare); } else if (!strcmp(cmd, "sortByText")){ qsort (mList, count, sizeof(message*), textCompare); } } return 0; }
Improve this question
2
  • See also SO 5228200. This is a different question, but the same data structures. Commented Mar 9, 2011 at 4:47
  • 1
    You might want to draw a picture of your mList structure and the operations you're doing on it, because what you're doing now is incoherent. Commented Mar 9, 2011 at 4:53

3 Answers 3

Reset to default
5

First off, your message list consists of 

message **mList

yet you are not using it as a pointer to a list of pointers. You never allocate the pointers in the list!

For example, this would be "correct" (though inefficient):

message **mList = malloc(INITIAL_CAPACITY * sizeof(message*)); for (i = 0; i < INITIAL; i++) { mList[i] = malloc(sizeof(message)); }

Now you have the data structure you've expressed.

However, you could also perform:

message *mList = malloc(INITIAL_CAPACITY * sizeof(message));

In all cases, once you have fixed your memory access problems, you can use realloc() to resize in place.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

At least from the looks of things, you're defining mlist incorrectly:

message **mList = malloc(INITIAL_CAPACITY * sizeof(message));

Since you're allocating the space for the message structures, it looks like mList should just be a message *:

message *mList = malloc(INITIAL_CAPACITY * sizeof(message));

or (what I usually prefer):

message *mList = malloc(INITIAL_CAPACITY * sizeof(*mList));

Overall, your code seems to be confusing two rather different cases. One is that you're dynamically allocating an array of messages, and the other is that you're allocating an array of pointers to messages. Though the question isn't an exact duplicate, I previously posted an answer with diagrams to show the difference graphically.

While the other answers are correct that realloc is generally useful for expanding an allocation, I think you need to do a bit more thinking about what you're expanding: an array of messages, or an array of pointers, with each actual message allocated individually. From the looks of things, your message structure itself contains a pointer to things like the actual text of the message, which is allocated separately from the message structure itself. That being the case, it probably makes more sense to allocate an array of message objects rather than an array of pointers.

Improve this answer

Comments

0

You need to use realloc() to make an allocated memory block larger.

realloc() will allocate a new block (if needed), copy the original data over to the new block, and free the old block (if needed).

If you just call malloc() again, you allocate a new memory block with new values. It also means you've created a memory leak because your original memory block doesn't get freed.

Improve this answer

Comments