A way to minimize errors and make your C code easier to read.
We’ve all been there, looking for a subtle bug in our code that is not trivial to find. The code at least compiles and run perfectly fine during most circumstances, alas someone elsewhere is reporting a corner-case we didn’t think of in the first place.
How do we reduce these bugs without changing our coding style or have to change under-laying logic?
There are a few best practices which I will write more on that will reduce the probability of this happening again and again. However, depending on your platform of choice and such some of the issues addressed might not look like an issue at all to you until you try running the very same code on a different platform/architecture/compiler. So we have that bias and we need to be aware of it, keeping in mind that even if the code will never run on say another CPU-architecture we should follow the principles to make it a habit of writing high-quality code. The old saying “bad code will live forever” is, of course, more pervading in embedded software than regular desktop application software where you might rewrite the code over the years, however, it doesn’t mean people won’t try to reuse your code in the most obscure ways.
After 25 years of coding and almost 15 years of working with development of embedded systems I’ve had my share of bad code, hardware issues, “cut, paste ‘n modify”-code that seems to work, typos, subtle errors and flawed logic. I even sometimes had to reverse engineer binary code from third parties to fix their bugs. What I’ve learned from these experiences is that many times the root of a bug doesn’t really look like a bug. When the code-base becomes larger and more complex it will, however, manifest as a bug.
So let’s start with with the most straightforward, but not obvious, parts first in this post and have more advanced examples in the upcoming articles. For this article, the reader is expected to have at least 2–3 years of professional (or equivalent) experience programming in C or C++.
Clever debugging
If you are not doing it already then use this in your format strings:
printf("%s:%d ...", __FUNCTION__, __LINE__, ...);This will show you from which function and which line-number in the code the debug-printout are at and should be supported by C99 and later. It will especially pay-off when using macros for/or dynamic debugging. If the project is large one might benefit even more by adding the filename to the debug-printout:
printf(“%s %s:%d …”, __FILE__, __FUNCTION__, __LINE__, …);How to declare things on the stack
Take a look at this trivial declaration of a buffer and spot what’s wrong:
char buf[100];99% of the time a declaration with that will work without any obvious issues.
However as one can see the buffer is not terminated, so let’s improve that with:
char buf[100] = {0};The zero-initializer will implicitly propagate to all elements and is equal to memset(buf, 0, (sizeof(buf)/sizeof(buf[0]))) which exemplifies another compiler trick which will be explained later. Pretty neat, however, there is still 1 thing wrong with the declaration. CPU cache-line sizes are usually a power of 2, on 32-bit MIPS-based system, it’s usually (but not always) 32 bytes. And since filling a whole cache-line is way more efficient than not doing so, which makes sense in IP-forwarding code and such (another example is the LZ4 compression algorithm), we should declare it as a power of 2:
char buf[128] = {0};Much better! For non-arrays / non-structs remember to initialize the variable to zero like:
int rc = 0;Having your variables always initialized reduce the number of undefined behaviours and helps catch errors in early stages of development.
Modifying strings and what can possibly go wrong?
When applicable use the C99’s “secure” string functions:
char buf[8] = {0};
sprintf(buf, "%s", "12345678blabla");The buffer will at best create a segmentation fault when writing the last argument. So let’s use snprintf():
char buf[8] = {0};
snprintf(buf, (sizeof(buf)/sizeof(buf[0])), "%s", "12345678blabla");What’s wrong here? It’s not overflowing the buffer! Well it’s not overflowing the buffer but it can easily do so, even worse the NUL delimiter in the buffer is prematurely terminating our buffer, which can lead to issues further down in the code path. We expected the buffer to hold the string “12345678” but it actually holds the string “1234567”.
A worse example on bad string-handling, which doesn’t have a NUL delimiter at all, but where the buffer holds “12345678” is the following:
char buf[8] = {0};
memcpy(buf, "12345678blabla", (sizeof(buf)/sizeof(buf[0])));So let’s correct the code:
char buf[8+1] = {0};
snprintf(buf, (sizeof(buf)/sizeof(buf[0])), "%s", "12345678blabla");This will make the buffer contain "12345678" + a NUL delimiter. Be aware of this extra byte needed when using snprintf() vs sprintf(), especially if you are improving some else’s code-base!
Now the funny pitfall that contradicts the lesson learned. Look at the following code (which is from code in production):
char macbuf[6] = {0};
unsigned int macb[] = {0x0, 0x11, 0x22, 0x33, 0x44, 0x55};
sprintf(macbuf, "%02x%02x%02x%02x%02x%02x", macb[0], macb[1], macb[2], macb[3], macb[4], macb[5]); What will happen if we add snprintf() and the extra byte here? Well, the simple answer is that you might get a corrupt MAC-layer if the buffer is used to build up an Ethernet frame, in this case the sprintf() is the valid function and snprintf() will just simply not work most of the time (an Ethernet address is 6 bytes and is also referred to as a MAC address).
Use what’s given to you
Most people won’t bother much about size_t and the signed version of it, ssize_t, which is a pity. This certainly decreases the portability of code, especially between 32-bit systems and 64-bit systems, and given different scenarios and integer might be too large (or too small) and become truncated and give bizarre bugs. So use the man pages and call the functions with what they expect if you can.
Let’s crunch some numbers!
Look at the following code:
int foobar(char *strptr) {
int i = 0;
i = atoi(strptr);
return i;
}What is returned if atoi() fails? Right a totally valid 0, which might be the number we just converted as well, so we want to be sure it doesn’t fail. Also there are more shortcoming of the old C89 atoi()/atof() functions, it doesn’t handle anything that’s not in base 10, hence "0x10" will return 0 since it will fail.
C99and newer luckily provides us with strtol()/strtod() to mitigate these problems. However one must remember to check ‘errno’ for any errors after using them:
long i = 0;
char *tmpptr = NULL;
int tmp_errno = 0:
i = strtol(strptr, &tmpptr, 10 /* We expect base 10 */);
tmp_errno = errno;... // Logic here might change 'errno' so we save it ASAP in the line above if (tmpptr == strptr) { // No conversion was done
...
} else if(tmp_errno == ERANGE) {
...
} else {
...
}
Also, note that we save the ‘errno’-variable as soon as possible to be able to call functions which might set ‘errno’ before doing the sanity checking. In this case, it’s most likely unnecessary but it’s a good habit that avoids errors that might be difficult to debug.
The page size and the stack - A love story
One common thing that usually hides memory corruption bugs and high stack pressure, especially in the kernel, is a large page size. For performance, it is usually to prefer a large page size, if configurable, but for systems with let’s say 64 MB of RAM the resources used will take up unnecessary memory, so the performance you gain might not be worth it. Well, remember that the Linux kernel’s stack is one buddy-allocated page size (that’s 2 pages that are next to each other). So what will happen if you modify the page size from 16 kilobytes to 4 kilobytes?
The kernel’s total stack will shrink dramatically from 32 kilobytes to 8 kilobytes; some WiFi-drivers allocate large 802.11-frames on the stack which might lead to a stack overflow manifesting during certain circumstances, albeit difficult to debug.
The page size will also affect user-space heap, since it’s rounded up to the closest multiplier of the page size, malloc(2048) will in reality in allocate 1 page, in this example 16 kilobytes or 4 kilobytes. In the former case, this gives you 14 kilobytes of memory that can be overwritten without causing noticeable issues, but in the latter case only 2 kilobytes.
If changing the page size is an issue then the code is poorly written and probably holds many other bugs. Test your code with the smallest page size available for this reason.
The eighties called and wanted alloca() back
Arcane and obscure and very little understood is the alloca() function, I sometimes use it personally to check an individual’s knowledge of C and Unix memory management by asking “How is alloca() implemented?”. Most people either don’t know or thinks it is related to the system calls for heap-allocation. These conclusions are common even among experienced people since it’s a rather complex implementation. Below I will give a brief explanation.
Usually being implemented in the compiler and used by the compiler (via in-lining) to make the stack “dynamic” it had some success 30 years ago, the advantage is that it will automatically deallocate the memory when it leaves the function from where it was allocated (technically it is not allocated or deallocated, but rather modifies the calling stack and it might overflow).
So successful that variable-length arrays (VLA), e.g. char buf[bufsize_variable];, based on the same principle become part of C99, luckily abandoned by later standards. While they can be allocated on the heap contrary to memory allocated with alloca(), GCC always uses the stack for them. The use case is typically an asynchronous signal handler where you want to dynamically allocate memory, however, it is better to use a fixed size these days. This doesn’t work well with heap-allocation and usually leads to invalid freeing of memory because the pointer changed and a new call was issued before the current had finished. alloca() being in-lined by default by the compiler (at least GCC does this) also gives unwanted side-effects if that might or might not break the expected behaviour.
Better use a compile-time buffer that is large enough to hold the data if you ever think of using alloca(), it will save you and others lots of headaches. Even if it is allocated on the stack, the OS will guarantee that the stack will be large enough when loading the executable.
The sizeof() operator
Now I will explain sizeof(buf)/sizeof(buf[0]) and when it makes sense. An array has at least 1 index at the first offset, the sizeof() will return the size, in the example, ‘buf’ is of type char, which is normally 1 byte, so it can be written like this since the prior sizeof() calculates the whole buffer-size on the stack with an array of 128 elements: (1*128)/(1*1).
Since this is not very useful for char-arrays, let’s use a short-array of 128 elements, sizeof(shrtarr)/sizeof(shrtarr[0]): (2*128)/(1*2). Here we can see how the division by The size of 1 short element is necessary to get the correct number of elements. But where there is simplicity there are pitfalls of course. Now, let's use it on a pointer!
short * shrtptr = (short * )malloc(128*sizeof(short));
size_t shrptr_arr_size = sizeof(shrtptr)/sizeof(shrtptr[0));In this example sizeof(shrtptr)/sizeof(shrtptr[0)) will return something totally different, most likely 1, since it will take the size of the pointer which is 2 and divide with 2. And if we want the size of what the pointer points to we have to write sizeof(*shrtptr) instead.
Lesson learned: Avoid using the sizeof-operator on pointers unless we are interested in what they point to, and even then you should avoid using it since the pointer might point to another pointer.
Summary and moving forward
The main ideas from the article can be summarized as follows:
- Try to sanitize your input by always making sure variables are correctly initialized
- Don’t allocate a large object on the stack when coding kernel code
- Check the return values and use functions that report errors if they do exist
- Use the C99 and newer alternatives for string manipulation over the old C89/ANSI C functions
- Don’t rely on implementation-specific behaviour
- Always consider the corner-cases
The examples in this article are trivial but it’s not always trivial to understand the implications they do have, especially not when it might work 99% of the time or even more. Changing the page size is not always possible and usually require a separate kernel and tool-chain, your mileage may vary; this advice is written from a perspective of embedded software development.
It is also of great importance to emphasize that you should not try to fix code that is proven to work even if it doesn’t follow the laid out principles unless you fully understand it. Much of the code in production systems is well-tested and working. There is always the risk of introducing unwanted regression errors when tinkering with it.
A good solution should always strive after simplicity and keep in mind the old saying:
“Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. Code for readability.”
As already mentioned the next article will handle more complex examples and situations.
