C Exception thrown when function is ran for a second time

Question

This is core of the function i am currently writing.

#include <cstdio>
#include <cstdlib>
#include <cctype>
#include <ctime>
#include <cstring>

const char *words[] = {
    "pigu",
    // "third",
    // "country",
    // "human",
    // "define",
};
#define word_count (sizeof(words) / sizeof(char *))
const char *allowed_chars = "abcdefghijklmnopqrstuvwxyz";

const char *get_random_word()
{
    return words[rand() % word_count];
}

char *copy(const char *origin)
{
    char *str = NULL;
    str = (char *)malloc(sizeof(origin));
    strcpy(str, origin);
    return str;
}

int run()
{
    const char *selected_word = get_random_word();
    char *active_word = copy(selected_word);
    char *placeholder_word = copy(selected_word);
    char *left_chars = copy(allowed_chars);

    free(active_word);
    free(placeholder_word);
    free(left_chars);
    return 1;
}

int main(int argc, char *argv[])
{
    srand(time(NULL));
    while (run())
    {
    }
    printf("\n");
    return 0;
}

I have stripped out other code and managed to locate the problem to run function, which in this case will run infinitely, however while debugging I have set break points inside run function. Turns out that after running the function run for second time, the program crashes when it tries to execute free(placeholder_word);. Why is this happening and how could i prevent it.

`sizeof origin` is not the length of the string--it's the size of a pointer, probably 8 bytes. Look up `strlen()` (and don't forget to add 1). — Lee Daniel Crocker, Feb 10 '18 at 01:33
@KeithThompson It first titled it C/C++ because I am using combination of c standard functions inside c++ wrapper libraries. However it has been edited by someone else. I was using g++ for compiling. — August, Feb 10 '18 at 12:05

Pablo · Accepted Answer · 2018-02-10T01:40:03.737

1

Your copy function is wrong, sizeof(origin) returns the number of bytes needed to store a pointer in memory, not the length of the string. So you've allocated the incorrect number of bytes and if the length of the string is longer than sizeof(origin) - 1, then you would overflow the buffer, which leads to undefined behaviour which would explain the segfault.

It should be

char *copy(const char *origin)
{
    char *str = NULL;
    str = malloc(strlen(origin) + 1);
    if(str == NULL)
        return NULL;
    strcpy(str, origin);
    return str;
}

Note that I've removed the cast of malloc, which is not needed in C. If you need it because this is a C++ program, the use new instead of malloc.

And you should always check if malloc returns NULL before accessing the memory.

edited Feb 10 '18 at 01:40

answered Feb 10 '18 at 01:27

Pablo

13,271
4
39
59

That whole function can be replaced with a call to `strdup`. – dbush Feb 10 '18 at 01:32
@dbush if the OP's system is a POSIX compliant, as `strdup` is not part of the standard C library. – Pablo Feb 10 '18 at 01:34
@Pablo Thank you I will accept this as soon as I can. The reason I was not using new, was because of the restrictions of my assignment. – August Feb 10 '18 at 01:34
@August in general, if you are compiling with a C++ compiler, use C++ code. If your C++ is only C code, then rename the file to `something.c` and compile it with the C compiler. Bear in mind that C is **not** a subset of C++. There are differences where C++ and C behave differently and when you expect some behaviour but get baffled because it's behaving strange. – Pablo Feb 10 '18 at 01:36

C Exception thrown when function is ran for a second time

1 Answers1