Common C Programming Mistakes
I taught C programming for many years and saw that some mistakes were fairly common, especially among people new to programming. All of these are mistakes according to the C89 standard (referred to as C89 below), but many are still wrong according to any C standard. Note that these specific error messages were generated when compiling with the GCC compiler, so the error messages you will see with a different compiler may vary from these.
You can find a video version of this with commentary at https://www.youtube.com/watch?v=WS8aHMK-a9c
Mistake #1: get the warning
ISO C90 forbids mixed declarations and code
/* WRONG */
#include <stdio.h>
int main(void) {
int x = 5;
printf("x = %d\n", x);
int z = 99; /* you can't declare here */
printf("z = %d\n", z);
}
/* RIGHT */
#include <stdio.h>
int main(void) {
int x = 5;
int z = 99;
printf("x = %d\n", x);
printf("z = %d\n", z);
}
Explanation: C89 requires that variables be declared at the top of a block, which is a set of curly braces. You can’t mix variable declarations with other types of statements.
Mistake #2: get the compilation error
expected expression before '/' token
/* WRONG */
#include <stdio.h>
int main(void) {
int x = 5;
printf("x = %d\n", x); // C++ style comment
}
/* RIGHT */
#include <stdio.h>
int main(void) {
int x = 5;
printf("x = %d\n", x); /* this IS a valid C89
comment style and
can span rows */
}
Explanation: C89 only has one type of comment tag. Comments must be begin with /* and end with */.
Mistake #3: get weird values
/* WRONG */
#include <stdio.h>
int main(void) {
int i, sum;
for(i = 1; i < 5; i++)
sum += i;
printf("%d\n", sum);
}
/* RIGHT */
#include <stdio.h>
int main(void) {
int i, sum = 0;
for(i = 1; i < 5; i++)
sum += i;
printf("%d\n", sum);
}
Explanation: The first program adds i to sum and re-assigns to sum on each iteration. The problem is that sum was never given an initial value, so we are adding i to the garbage value that sum begins with. The solution is to initialize sum to whatever you want it to begin with. What makes this error even worse is that sometimes when you run your program, the initial garbage value for sum might be 0 which results in the correct value during that run.
Mistake #4: using = instead of ==
/* WRONG */
#include <stdio.h>
int main(void) {
int x = 5;
if(x = 99) /* only a single equal sign */
printf("prints this");
else
printf("doesn't print");
}
Explanation: When you write
x = 99
you are assigning a value of 99 to x; the overall statement has a value of 99, which is TRUE. If you really want to check if x already has a value of 99 and then do something if this statement is TRUE, then you should use
x == 99
Mistake #5: using int d[] instead of int d[10]
/* WRONG */
#include <stdio.h>
int main(void) {
int d[]; /* error occurs here */
d[0] = 99;
}
/* RIGHT */
#include <stdio.h>
int main(void) {
int d[10];
int x[] = {1, 2, 3};
d[0] = 99;
}
Explanation: The first program has the square brackets required for statically allocated arrays, but we never tell the computer how much memory to allocate. The second program demonstrates two different ways of letting the computer know how much memory to allocate. Array d explicitly tells the computer to allocate enough memory to store 10 ints. Array x leaves the square brackets empty, so the computer will count how many ints we have provided and will allocate just enough memory for these ints.
Mistake #6: applying sizeof to a passed array
/* WRONG */
#include <stdio.h>
void fx(int d[]) {
int i;
int cols = sizeof(d)/sizeof(d[0]);
for(i = 0; i < cols; i++)
printf("%d", d[i]);
}
int main(void) {
int d[] = {1, 2, 3, 4};
fx(d);
}
/* RIGHT */
#include <stdio.h>
void fx(int d[], int cols) {
int i;
for(i = 0; i < cols; i++)
printf("%d ", d[i]);
}
int main(void) {
int d[] = {1, 2, 3, 4};
int cols = sizeof(d)/sizeof(d[0]);
fx(d, cols);
}
Explanation: When we ‘pass’ an array to a function, we are really passing the address of the first element of the array. Therefore, when we try to use sizeof inside the function to determine the overall size of the array, we are really getting the size of an address.
Mistake #7: changing passed variables from a function (this is a logic issue)
/* WRONG */
#include <stdio.h>
void fx(int x) {
x = x + 99;
}
int main(void) {
int x = 5;
fx(x);
printf("x is now %d\n", x); /* prints 5 */
}
/* RIGHT */
#include <stdio.h>
void fx(int* x) {
*x = *x + 99; /* a pointer is used to
change the x in main */
}
int main(void) {
int x = 5;
fx(&x);
printf("x is now %d\n", x); /* prints 104 */
}
Explanation: The first program passes x to the function fx() with the goal of changing x. The problem is that the x declared in main() and the x declared in fx() are two different variables, with the initial value of the x declared in fx() being the value of the x declared in main().
Because they are two different variables, changes to the x in fx() do not affect the x in main().
Mistake #8: changing passed pointer from a function
/* WRONG */
#include <stdio.h>
void fx(int* p, int d[]) {
p = &d[1];
}
int main(void) {
int d[] = {1, 99};
int* ptr = &d[0];
printf("*ptr is %d\n", *ptr); /* *ptr is 1 */
fx(ptr, d);
printf("*ptr is now %d\n", *ptr); /* *ptr is now 1 */
}
/* RIGHT */
#include <stdio.h>
void fx(int** p, int d[]) {
*p = &d[1];
}
int main(void) {
int d[] = {1, 99};
int* ptr = &d[0];
printf("*ptr is %d\n", *ptr); /* *ptr is 1 */
fx(&ptr, d); /* pass address of ptr */
printf("*ptr is now %d\n", *ptr); /* *ptr is now 99 */
}
Explanation: This is the same problem as the previous mistake. We want to change the value of the pointer created in main(), which means that we need its address. This requires a pointer to a pointer in fx().
Mistake #9: using char*s instead of char s[20] and then trying to change s[0]
/* WRONG */
#include <stdio.h>
int main(void) {
char* s = "cat";
s[0] = 'X';
printf("%s\n", s);
}
/* RIGHT */
#include <stdio.h>
int main(void) {
char s[20] = "cat";
s[0] = 'X';
printf("%s\n", s);
}
Explanation: In the first program, s is a string literal and contains the address of the c in cat. The C standard is fuzzy about what should happen if we try to change the contents of this string, so we should treat it as being constant. We can change the address stored in s to point to a different string, but we can’t change the original string itself. The second program allocates space for an array of 20 characters, any of which we can change.
Mistake #10: passing incorrect values to strcmp()
/* WRONG */
#include <stdio.h>
#include <string.h>
int main(void) {
char text[] = "X";
if( strcmp(text, 'X') == 0 ) /* error is here */
printf("the strings are equal\n");
else
printf("the strings are not equal\n");
}
/* RIGHT */
#include <stdio.h>
#include <string.h>
int main(void) {
char text[] = "X";
if( strcmp(text, "X") == 0 )
printf("the strings are equal\n");
else
printf("the strings are not equal\n");
}
Explanation: strcmp() expects two strings (i.e., null-terminated arrays of characters). These are enclosed by double quotes. A single character enclosed by single quotes is just that: a single character, which is a type of int.
Mistake #11: attempting to increment a value being pointed to with *ptr++
/* WRONG */
#include <stdio.h>
#include <string.h>
int main(void) {
int x = 99;
int* ptr = &x;
printf("ptr = %p and *ptr = %d\n", ptr, x);
*ptr++;
printf("ptr = %p and *ptr = %d\n", ptr, x);
}
/*
ptr = 0x7fff617a4e84 and *ptr = 99
ptr = 0x7fff617a4e88 and *ptr = 99
*/
/* RIGHT */
#include <stdio.h>
#include <string.h>
int main(void) {
int x = 99;
int* ptr = &x;
printf("ptr = %p and *ptr = %d\n", ptr, x);
*ptr = *ptr + 1;
printf("ptr = %p and *ptr = %d\n", ptr, x);
}
/*
ptr = 0x7ffffd6ac0d4 and *ptr = 99
ptr = 0x7ffffd6ac0d4 and *ptr = 100
*/
Explanation: It is legal to write *ptr++. The problem, in this situation, is that due to the precedence of operators, *ptr++ says to use what is currently pointed to and then increment the address stored in the pointer when what you really want is to increment the value (i.e., the integer in this case) being pointed at.
Mistake #12: using char*a instead of char a[20]; this is a memory storage issue
/* WRONG */
#include <stdio.h>
#include <string.h>
int main(void) {
char * temp;
char s[] = "this is a long string";
strcpy(temp, s);
printf("%s", temp);
}
/* RIGHT */
#include <stdio.h>
#include <string.h>
int main(void) {
char temp[50];
char s[] = "this is a long string";
strcpy(temp, s);
printf("%s", temp);
}
Explanation: In the first program, temp is of type char* but uninitialized and therefore contains garbage. The program attempts to copy the string in s to the address stored in temp. The second program allocates enough space in temp to store 50 characters.
Mistake #13: copying a string into dynamically-allocated space
/* WRONG */
#include <stdio.h>
#include <stdlib.h>
int main(void) {
char* text = malloc(50 * sizeof(char) );
char buffer[] = "this is a string";
/* attempt to copy buffer into text */
text = buffer;
}
/* RIGHT */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(void) {
char* text = malloc(50 * sizeof(char) );
char buffer[] = "this is a string";
strcpy(text, buffer);
}
Explanation: In the first program, the address of buffer is stored in text and the address of the dynamically allocated memory is lost.
Mistake #14: storing an address versus what is at the address; this is a memory storage issue
/* WRONG */
#include <stdio.h>
#include <string.h>
void readFile(FILE *fp) {
char buffer[100];
char* firsts[5];
char* del = " ";
for(i = 0; i < 5; i++) {
fgets(buffer, sizeof(buffer), fp);
firsts[i] = strtok(buffer, del);
}
}
/* RIGHT */
#include <stdio.h>
#include <string.h>
void f(FILE *fp) {
char buffer[100];
char* firsts[5];
char *del = " ", *token;
for(i = 0; i < 5; i++) {
fgets(buffer, sizeof(buffer), fp);
token = strtok(buffer, del);
firsts[i] = malloc( strlen(token) + 1 );
strcpy(firsts[i], token);
}
}
Explanation: The first program attempts to store the first word of each line, but really only stores the address of the first token of each line. This address is to a position in buffer and likely is to the first character of the array. The second program allocates space to store a word, saves its address in the array of addresses, and then copies the token to this space.