CODEWITHSUNDEEP
c
MichaelX
MichaelX

Reputation: 202

Realloc is messing up the code

I wrote a program that's supposed to do a find and replace inside a file, with the condition that the new word must be longer than the old one. I had a lot of issues with it, like printing weird characters, or having the core dumped, but when I removed the realloc statement the problems seemed to go away. I'm not very familiar with the function, what mistake did I do?

Also, are my frees where they should be?

void replace_word(const char *s, const char *old_word, const char *new_word){
    char *buffer = malloc(BUFFER_SIZE);
    char *init = buffer;
    FILE *original_file;
    FILE *copy;

    if((original_file = fopen(s, "r")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }

    if((copy = fopen("copy.txt", "w")) == NULL){
        perror("text");
        exit(EXIT_FAILURE);
    }

    int old_word_len = strlen(old_word);
    int new_word_len = strlen(new_word);

    char *src;
    char *dst;
    char *tmp;
    while(fgets(buffer, BUFFER_SIZE, original_file)){
        if((tmp = strstr(buffer, old_word))){
            buffer = realloc(buffer, BUFFER_SIZE + new_word_len - old_word_len);
            src = tmp + old_word_len;
            dst = tmp + new_word_len;
            memmove(dst, src, strlen(src));
            memcpy(tmp, new_word, new_word_len);
            buffer = init;
        }
        fprintf(copy, "%s", buffer);
        if(!strchr(buffer, '\n')){
            fseek(original_file, -old_word_len, SEEK_CUR);
        }
        free(buffer);
        buffer = malloc(BUFFER_SIZE);
    }
    free(buffer);
}

Upvotes: 1

Views: 183

Answers (1)

Elan Hamburger
Elan Hamburger

Reputation: 2177

realloc takes a pointer to dynamically assigned memory and resizes the memory block. An important point that people less experienced with dynamic memory often miss is that the pointer returned by realloc may not be the same as the pointer passed in (and usually isn't). Generally, the heap does not have enough contiguous memory to do the requested expansion, so it finds a new block of memory large enough to fit it and copies the previous contents in.

The first thing I notice about your code is that you perform an unsafe realloc:

buffer = realloc(buffer, BUFFER_SIZE + new_word_len - old_word_len);

By assigning a pointer to its own realloc, you run the risk that the OS cannot complete the allocation and returns NULL. If that occurs, you've lost the pointer to your old memory and you have a memory leak on your hands. To properly realloc (and I'm guessing you'll need to do this anyway to fix my next point), assign the results of realloc to a different pointer, confirm it's not NULL, and then overwrite the pointer.

But I think the main problem with your code lies here:

buffer = init;

If I understand correctly, the purpose of this statement is to return buffer to the beginning of the string. However, the memory that init pointed to has been freed by the realloc, and you end up dereferencing the dangling pointer. You'll need to store a reference to the beginning of the new memory returned by realloc.

Upvotes: 2

Related Questions