Sponsored by Rogue Wave, maker of Klocwork

How good are you at finding errors in code? This is our second bug for you to track down and eradicate. The following is a code snippet compiles fine. Unfortunately, it contains an error. Take a look at the code listing and see if you can find the problem. To make it easier, we've listed five options. One of those options is correct while the other four are not.

Can you find the error without looking at the choices?

Bug Buster #2

Here is the Bug Buster:

The code:

#include <stdio.h>
#include <stdlib.h>

/*======================================*/
/* Print "words" made of random letters */
/*======================================*/

int main()
{    
    int len = 10;    /* length of each string */
    int amount = 5;  /* number of strings */
    int x, y;        /* simple counters */
    char * buffer;   /* buffer to store random string */

    /* create the random strings */
    for(x = 0; x < amount; x++)
    {
        /* allocate memory for string */
        buffer = (char*)malloc(len + 1);
        if (buffer == NULL) exit(1);  
 
        /* build the random string */
        for (y = 0; y < len; y++)
            buffer[y] = rand() % 26 + 'a';
        buffer[len] = '\0';
 
        /* print the random string*/
        printf("Random string: %s\n", buffer);
    }   
 
    return 0;
}

Your choices:

How quickly can you find the issue? Feel free to comment on how quickly you found the issue! We will be posting additional snippets over the coming weeks with additional bugs for you to bust. You can click to the next page to find the solution.

There are five options to pick from regarding this listing. Although the listing will compile and will run just fine, there is a bug that needs to be eradicated to avoid issues. As such, that leaves only four options to choose from. There is nothing wrong with comparing the allocated buffer to NULL. In fact, you should do this to make sure your memory was allocated and ready for you to use. Because there is nothing wrong with the allocation, the malloc should work unless your system is already deprived of memory. Your buffer should generally be allocated. As to buffer overflows, this listing should be okay.

That leaves the fourth choice, the dreaded memory leak. This program definitely has a memory leak. Even though you might never notice it with a program that only allocates a few character arrays, if this program worked with thousands of strings, it could get to a point where it uses all of the system's memory. The program allocates strings with malloc, but it never frees that memory. The listing should have a free statement added as such:

#include <stdio.h> 
#include <stdlib.h>
 
/*======================================*/
/* Print "words" made of random letters */
/*======================================*/
int main()
{
    int len = 10;    /* length of each string */
    int amount = 5;  /* number of strings */
    int x, y;        /* simple counters */
    char * buffer;   /* buffer to store random string */
 
    /* create the random strings */
    for(x = 0; x < amount; x++)
    {
        /* allocate memory for string */
        buffer = (char*)malloc(len + 1);
        if (buffer == NULL) exit(1);  
 
        /* build the random string */
        for (y = 0; y < len; y++)
            buffer[y] = rand() % 26 + 'a';
        buffer[len] = '\0';
 
        /* print the random string*/
        printf("Random string: %s\n", buffer);
 
        free(buffer);
    }   
 
    return 0;
}

You always need to release the memory.

This program also has another issue. This is not a bug, but rather simply poor programming. Instead of allocating and freeing the memory each time a new string is created, it would have been better to do this once. The allocation could have been done before the loop and then a single free could have been done after. The following shows this small, but important, change:

#include <stdio.h> 
#include <stdlib.h>
 
/*======================================*/
/* Print "words" made of random letters */
/*======================================*/
int main()
{
    int len = 10;    /* length of each string */
    int amount = 5;  /* number of strings */
    int x, y;        /* simple counters */
    char * buffer;   /* buffer to store random string */
 
    /* allocate memory for string */
    buffer = (char*)malloc(len + 1);
    if (buffer == NULL) exit(1);  
 
    /* create the random strings */
    for(x = 0; x < amount; x++)
    {
 
        /* build the random string */
        for (y = 0; y < len; y++)
            buffer[y] = rand() % 26 + 'a';
        buffer[len] = '\0';
 
        /* print the random string*/
        printf("Random string: %s\n", buffer);
    }   
    free(buffer);
 
    return 0;
}

This memory leak could easily have gone unnoticed because there were no compile errors. In a program allocating bigger amounts of memory or more elements, you risk crashing the entire computer. Memory leaks like this, however, are just one of the errors that a static code analysis tool like Rogue Wave's Klocwork can find quickly, thus saving you embarrassment and time later.

It is also worth noting that using more modern coding standards for allocating memory could also help to reduce the chances of creating memory leaks.