strcpy corrupts char array (string value)

Question

The function below tries to order strings on a linked list in ascending order. When it returns the new list, it becomes corrupted.

void* order( void *ptr){
    struct wordlist *head;
    head = (struct wordlist *) ptr;

    struct wordlist *first = (struct wordlist*)malloc(sizeof(struct wordlist));
    struct wordlist *second = (struct wordlist*)malloc(sizeof(struct wordlist));
    struct wordlist *temp = (struct wordlist*)malloc(sizeof(struct wordlist));

    first = head;

    int j = 1;
    while( first != NULL){
        second = first->next;

        while( second != NULL){
            if( strcmp( first->word, second->word) > 0){
                if( temp->word == NULL){
                    temp->word = malloc( sizeof(first->word));
                }
                else{
                    if( realloc( temp->word, sizeof( first->word)) != NULL){
                        strcpy( temp->word, first->word);
                    }
                }

                if( realloc( first->word, sizeof(second->word)) != NULL){
                    strcpy( first->word, second->word);
                }

                if( realloc( second->word, sizeof(temp->word)) != NULL){
                    strcpy( second->word, temp->word);
                }

                free(temp);
            }
            second = second->next;
        }
        j++;
        first = first->next;
    }
}

For example, if input would be

piero
ronaldo
messi

then the output looks like

messi
ŽŽŽ
ronaldo

The above example is not tried on the code, but it will give you a clue. I believe there is something with the allocation of the memory but I could not manage to find it. By the way, sometimes the words come empty too.

Also, the wordlist is as follows:

struct wordlist{
    char *word;
    struct wordlist *next;
};

To order a linked list you shouldn't need to do all this memory allocation, you should be able to just change some `next` pointers around whenever you want to move something. On a related note, you are allocating memory to `second`, then you are leaking this immediately with `second = first->next;` — lxop, Mar 27 '13 at 02:21
You do realise that you can just swap pointers, right? You don't have to `realloc` and `strcpy` to move them around. — paddy, Mar 27 '13 at 02:23

paddy · Accepted Answer · 2013-03-27T02:34:57.213

You don't copy the string into your temporary the first time around.

            if( temp->word == NULL){
                temp->word = malloc( sizeof(first->word));
                // You forgot to copy!!
            }
            else{
                if( realloc( temp->word, sizeof( first->word)) != NULL){
                    strcpy( temp->word, first->word);
                }
            }

See, if temp->word is NULL, which it ought to be the first time (note that you don't actually clear the temp struct so already you will get undefined behaviour), then you don't copy it. The quick fix is to do a strcpy after you malloc.

Your realloc calls are all wrong. You cannot use sizeof to get the size of a string. Use strlen for that, and don't forget to add an extra byte for the string terminator.

Furthermore, you should not allocate first and second. They are iterators for your data structure. The first thing you do is discard their value so you leak memory. Don't forget to free your temp structure as well as temp->word afterwards.

After you get that working, please stop all this malloc and strcpy business!!!

To move the strings around you only need to move the pointers. No reallocation or copies required. This will simplify your code down to a handful of lines.

Oh, and did you also forget to return a value from your function?

I just deleted everything inside the "if strcmp {}" part and did a pointer swap in three lines. It works like a charm. I wasted hours to fix this but it comes out so simple. Thank you. — gzg, Mar 27 '13 at 02:35
No problem. It was not wasted -- hopefully you learned a lot from the experience =) — paddy, Mar 27 '13 at 02:37

strcpy corrupts char array (string value)

1 Answers1