5 Deadly Mistakes to Avoid in C Programming

Table of Contents

Introduction

C programming mistakes could be difficult to troubleshoot if you are not fully aware of the principles behind this language. It is possible to spend a great deal of time just to find a careless typo in your code that takes 2 seconds to fix. Frustrated you may be, but this is all part of the learning curve.

I believe it is the best to avoid deadly mistakes rather than spending the time to find your own deadly mistakes. If you know what you are doing while you code, the less time you will spend debugging it. Here, I would like to share 5 common deadly mistakes that you can avoid today.

Incorrect Use of sizeof() and strlen()

Size is very important in C programming because it is a lower level language and the compiler may not necessarily know the size of a variable of structure that you are working with. It is YOUR responsibility to fully aware of the sizes. One of the most commonly mistakes is to misuse “sizeof()” and “strlen()”.

Keep this in mind:

use sizeof() to return the size a variable occupies in memory.
sizeof() does NOT return the number of elements in an array.
use strlen() to return the length of a string (aka. an array of char terminated by \0).
strlen() returns incorrect value if the string is not NULL-terminated.

Consider this example:

void sizeof_mistake(void)
{
	char 	myCharArray[20];
	int 	myIntArray[20];
	char *	myCharPtr = "abcde";
	int 	i = 0;

	for (i = 0; i < sizeof(myCharArray); i++)
	{
		myCharArray[i] = 'a';
	}

	for (i = 0; i < sizeof(myIntArray); i++)
	{
		myIntArray[i] = 'a';
	}

	strncpy(myCharArray, myCharPtr, sizeof(myCharPtr));
}

Can you spot the mistakes?

The first for loop is correct, but not recommended. “sizeof(myCharArray)” returns 20, which equals the size of the array. This works because the data type is char, which occupies 1 byte in memory and we have 20 of them, so sizeof returns 20. The recommended way to iterate this array is simply replace i < sizeof(myCharArray) with i < 20 or put 20 in a MACRO instead.

/* Loop through myCharArray 20 times - correct but not recommended */
for (i = 0; i < sizeof(myCharArray); i++)
{
	myCharArray[i] = 'a';
}

The second for loop, on the other hand, is incorrect. myIntArray also contains 20 elements, but it has a data type of int, which occupies 4 bytes in memory. We have 20 of it, so in total it occupies 80 bytes in memory. If you mistakenly use sizeof(myIntArray) to iterate through this array, your program will crash. The correct way is to replace sizeof(myIntArray) with simply 20 or put 20 in a MACRO instead.

/* INCORRECT: This will Loop through myIntArray 80 times! */	
for (i = 0; i < sizeof(myIntArray); i++)
{
	myIntArray[i] = 'a';
}

This line of string copy (strncpy) statement is also incorrect, because myCharPtr is a pointer data type, which contains a memory address and always occupies 8 bytes in memory. myCharPtr points to a string of length 5 (“abcde”) which is less than 8, so using the copy statement as is will result in a crash as well.

/* INCORRECT: sizeof(myCharPtr) returns 8 bytes, which is the size of pointer, not the length of the string */
strncpy(myCharArray, myCharPtr, sizeof(myCharPtr));

The correct way to copy the string from myCharPtr to myCharArray is to use strlen() instead of sizeof()

strncpy(myCharArray, myCharPtr, strlen(myCharPtr));

Compare Pointer Instead of Value

Be careful when you work with pointers. Please keep the following in mind:

Are you working with reference?
or are you working with values?

Consider this example:

void compare_ptr_mistake(void)
{
	int myNum1 = 200;
	int myNum2 = 200;

	int * myNum1ptr = &myNum1;
	int * myNum2ptr = &myNum2;

	if(myNum1ptr == myNum2ptr)
		printf("myNum1ptr == myNum2ptr\n");
	else
		printf("myNum1ptr != myNum2ptr\n");
}

Can you guess what the above example will print? The above example will produce the output below:

myNum1ptr != myNum2ptr

and indeed myNum1ptr and myNum2ptr are different even though they point to the same value of 200. They are different because the if statement compares the references instead of their values. Since they point to different memory locations, they are considered different.

If we were to compare the values pointed by the pointers, we need to use the dereferencing operator (*). The following example, in this case, will print “myNum1ptr == myNum2ptr”.

	if(*myNum1ptr == *myNum2ptr)
		printf("myNum1ptr == myNum2ptr\n");
	else
		printf("myNum1ptr != myNum2ptr\n");

Forget to Allocate Extra 1 Byte For Terminating NULL Character

We all know that strings in C programming have to be null-terminated. What if you forget about it? Hard to say, if you are lucky, the program will just crash. If you are unlucky, the program goes on causing other random issues that are impossible to make sense of (Really painful to debug).

Consider this example below where we have a char buffer of 20, set it to all zero first with memset(), give it a value of “11111222223333344444” using strncpy and finally print out the string and its length. This function may or may not work as intended because there is no room in myString char array to hold the extra terminating NULL character (\0).

void forget_terminating_null_mistake(void)
{
	char myString[20];

	memset(myString, 0, sizeof(myString));
	strncpy(myString, "111112222233333344444", 20);
	printf("myString = %s, myString length = %lu\n", myString, strlen(myString));
}

The printf() function will continue to print the value of myString until it finds a terminating NULL character. This could mean going out of bound of the myString array and potentially print garbage values out. The same applies to strlen() function where it will also iterate out of bound until it finds a terminating NULL character, thus producing an incorrect length.

To fix, we simply declare myString with 1 additional space for terminating NULL:

char myString[20+1];

Struct with Pointers

Working with pointers requires more care, especially with struct pointers. This is one common mistake people make when working with struct pointers. Consider the example below:

typedef struct structB
{
	int id;
} StructB;

typedef struct structA
{
	int id;
	StructB * b;
} StructA;
void struct_containing_pointer_mistake(void)
{
	StructA * a;
	a = (StructA*) malloc(sizeof(StructA));
	if (!a)
	{
		printf("failed to allocate memory\n");
		return;
	}
	memset(a, 0, sizeof(StructA));

	a->id = 500;
	a->b->id = 600;

	if (a)
		free(a);
	if (b)
		free(b);
}

Can you spot the mistake? Let’s walk this through.

We allocate a StructA pointer, allocate heap memory for it using malloc() call and we specify the requested size using sizeof(StructA), which should equal to the total size StructA data type occupies in memory, including the StructB pointer that is inside.

While the above is true, the example problem above will crash at this line below, due to invalid memory access even though we have just allocated memory for it.

a->b->id = 600;

If fact, what we have allocated is just the StructB pointer itself (8 bytes to store memory address) but we have not specify where StructB pointer inside StructA should point to, so it points to nothing in the example and therefore the program will crash when trying to modify it. We can adjust the program like below to avoid the crash:

typedef struct structB
{
	int id;
} StructB;

typedef struct structA
{
	int id;
	StructB * b;
} StructA;
void struct_containing_pointer_mistake(void)
{
	StructA * a;
	a = (StructA*) malloc(sizeof(StructA));
	if (!a)
	{
		printf("failed to allocate memory\n");
		return;
	}
	memset(a, 0, sizeof(StructA));

	a->id = 500;
	a->b = (StructB*) malloc(sizeof(StructB));
	if (!a->b)
	{
		printf("failed to allocate memory\n");
		return;
	}
	a->b->id = 600;

	if (a)
		free(a);
	if (b)
		free(b);
}

Free Memory at Wrong Address

Memory management in C programming is notoriously tedious, because it forces you to be aware of the memory you have allocated and forced you to free it when not in use. You may remember to free what you allocate, but what if you free the wrong memory by accident? Your program will abort if doing so. Let’s take a look by consider the example below where arithmetic operations are done on the pointer:

void free_pointer_mistake(void)
{
	unsigned char * buffer_ptr = NULL;
	int myint1 = 200;
	int myint2 = 600;

	buffer_ptr = (unsigned char*) malloc (sizeof(char) * 8);
	if (!buffer_ptr)
	{
		printf("failed to allocate memory\n");
		return;
	}
	/* store myint1 to buffer */
	memcpy(buffer_ptr, &myint1, sizeof(int));
	buffer_ptr += sizeof(int);

	/* store myint2 to buffer */
	memcpy(buffer_ptr, &myint2, sizeof(int));

	/* free the pointer when done */
	if (buffer_ptr)
	{
		free(buffer_ptr);
		buffer_ptr = NULL;
	}
}

Can you spot the mistake here? The example allocate 8 bytes of memory for buffer_ptr to hold 2 integers (4 bytes each). To do so, the example uses memcpy() to copy myint1 to the buffer, advance the pointer by 4 bytes, and uses memcpy() again to copy myint2. So far so good. The problem occurs when we try to free buffer_ptr at this line:

free(buffer_ptr);

The program will abort when the above line is run, because buffer_ptr no longer points to the address where 8 bytes of memory is allocated by the malloc() call. The pointer is changed by line buffer_ptr += sizeof(int) because we are using pointer arithmetic to copy integer data. Such an easy-to-miss mistake.

How do we fix? Simple! We basically have to remember the address where the allocation is made and free() that address instead. The example below fixes the issue by introducing another pointer called tmp.

void free_pointer_mistake(void)
{
	unsigned char * buffer_ptr = NULL;
	unsigned char * tmp = NULL;
	int myint1 = 200;
	int myint2 = 600;

	buffer_ptr = (unsigned char*) malloc (sizeof(char) * 8);
	if (!buffer_ptr)
	{
		printf("failed to allocate memory\n");
		return;
	}
	/* remember this address before we do pointer arithmetic */
	tmp = buffer_ptr;

	/* store myint1 to buffer */
	memcpy(buffer_ptr, &myint1, sizeof(int));
	buffer_ptr += sizeof(int);

	/* store myint2 to buffer */
	memcpy(buffer_ptr, &myint2, sizeof(int));

	if (tmp)
	{
		/* remember tmp instead of buffer_ptr */
		free(tmp);
		tmp = NULL;
	}
}

Summary

We have gone over 5 common but deadly mistakes in C programming that you should avoid to save time from debugging. I hope that you have learned a thing or 2 from this article and if you have any questions, feel free to contact me.