Search code examples
cmemory-managementrealloc

Reverse string realloc(): invalid next size


I have created this small program to reverse a sentence. So given: a b c d

It will give: d c b a

This works fine, until I add an extra letter. If I try "a b c d e" it will fail at the last one with the error

realloc(): invalid next size: 0x0000000000602010 *

Here is the code

#define MAX_TEXT 100

#include <stdio.h>
#include <string.h> //strlen
#include <stdlib.h> //realloc


int main(int argc, char **argv) {
    char text[MAX_TEXT] = { 0 };    
    char *parts = NULL;

    printf("Insert string: ");
    fgets(text,MAX_TEXT,stdin);
    sscanf(text,"%[^\n]",text); //remove the \n


    char **reverse = NULL;
    char **extra = NULL;
    int size = 0;
    int i = 0;

    parts = strtok (text," ");
    while (parts != NULL) {

        size += ((strlen(parts)+4) * sizeof(char));
        extra = realloc(reverse,size);

        if (extra) {
            reverse = extra; 
            reverse[i++] = parts;
        }     
        else {
            printf("Error allocating memory\n");
            exit(1);
        }

        parts = strtok (NULL, " ");
    }

    while (--i >= 0) {
        printf("%s ",reverse[i]);                
    }

    printf("\n");
}

I am still newbish with using pointers let alone pointer to pointer, so any help will be great. Thanks!

Ps: I tried using valgrind and it just points that there is something wrong at the realloc but I can't understand what exactly.

UPDATED CODE:

#define MAX_TEXT 500

#include <stdio.h>
#include <string.h> //strchr
#include <stdlib.h> //realloc


int main(int argc, char **argv) {
    char text[MAX_TEXT] = { 0 };        

    printf("Insert string: ");
    fgets(text,MAX_TEXT,stdin);
    //sscanf(text,"%[^\n]",text); //remove the \n <-- Undefined behaviour :D "if copying takes place between objects that overlap, the behavior is undefined."
    char *theEnter = strchr(text,'\n');
    if (theEnter) {
        *theEnter = 0;//remove \n
    }        

    char **reverse = NULL;
    char **extra = NULL;
    char *parts = NULL;
    int size = 0;
    int i = 0;
    size_t increse_by = sizeof(char *);

    parts = strtok (text," ");
    while (parts != NULL) {

        size += increse_by; //pointer to pointer so increase by the size of new pointer
        extra = realloc(reverse,size);

        if (extra) {
            reverse = extra; 
            reverse[i++] = parts;
        }     
        else {
            printf("Error allocating memory\n");
            exit(1);
        }

        parts = strtok (NULL, " ");
    }

    while (--i >= 0) {
        printf("%s ",reverse[i]);                
    }

    printf("\n");
}

Following Charlie Burns indication I am now allocating for char *, I also removed the sscanf function and used strchr to remove \n because, as chux indicated, what I was doing was undefined behavior.

Thanks for the help :)


Solution

  • Your realloc is not allocating enough space. sizeof(char *) is probably 8 on your machine. Your realloc below will allocate (1 + 4) * 1 = 5 for a parts of strlen == 1. That's not enough for the string pointer.

    Try changing

        size += ((strlen(parts)+4) * sizeof(char));
    

    to

        size += sizeof(char *);
    

    Reason being that extra and reverse are char **. So they are an array of pointers to strings. This array gets one bigger with each loop through strtok(). strtok() returns a null terminated string. So you don't need to alloc memory for that.

    You don't need to in this case, but you could also do:

    reverse[i++] = strdup(parts);
    

    If you wanted to the strings in reverse to point to copies of the strings in text, not inside text itself.