Search code examples
cstringpointersfor-loopstrcat

strcat crashes program (0xc0000005)


I need to draw a line of characters as long I want. So I wrote a function for that purpose:

void fDrawLine(int length)
{
    int i;
    char * compLine = (char *) malloc(WINDOW_WIDTH + 2);

    for(i = 0; i < length; i++)
        strcat(compLine, "-");

    fDrawSpacedMessage(compLine, -1, TRUE);
}

WINDOW_WIDTH defined as 80, fDrawSpacedMessage is another function to print texts centered etc.

It's building perfectly, no errors, no warnings. But in runtime, everything works but if fDrawLine executes, the program crashes and gives the error code 0xc0000005. I know it's about memory allocation but I already initialize the compLine string.

I've tried a couple of things; I thought another function caused it so I isolated fDrawLine, but crashing continued. Changing initialize with compLine[0] = 0;, compLine[WINDOW_WIDTH] = {0}; did not help.

It works well with my other machine, which runs Ubuntu, with latest gcc, but when using Code::Blocks (MinGW) on Windows it keeps crashing.

What's wrong with this code?


Solution

  • Don't declare compLine as a pointer, since you don't need that, and actually you have a memory leak in your function, first declare compLine this way

    char compLine[1 + WINDOW_WIDTH] = {0}; // strings need an extra byte at the end to mark the end.
    

    then use memset to set the '-' character like this

    memset(compLine, '-', length);
    

    of course, check that length <= WINDOW_WIDTH.

    This is your function fixed, so you can try it

    void fDrawLine(int length)
    {
        char compLine[1 + WINDOW_WIDTH] = {0}; // initialized so that last byte is '\0'.
        if (length > WINDOW_WIDTH)
            length = WINDOW_WIDTH;
        memset(compLine, '-', length);        
        fDrawSpacedMessage(compLine, -1, TRUE);
    }
    

    besides using strcat that way is a bad idea, you can do it this

    char *compLine = malloc(1 + length); // the last extra '\0' byte.
    if (compLine == NULL) // malloc returns NULL on failure to allocate memory
        return; // so we must abort this function in that case.
    for(i = 0; i < length; i++)
        compLine[i] = '-';
    compLine[length] = '\0';
    
    fDrawSpacedMessage(compLine, -1, TRUE);
    free(compLine);
    

    you can also use memset in this case, and it is actually better.