Ease of screwing up C string operations.
Posted by peeterjoot on August 2, 2012
Here’s a new way to mess up a strncpy that I hadn’t seen before. I found about 50 instances of this in the component I was working on today:
ossStrNCopy( pszTarget, pszSource, strlen( pszSource ) + 1 ) ;
Our ossStrNCopy function is a lot like strncpy, but unlike the library interface, ours is explicitly truncating and won’t leave a dangling non-null terminated string. It’s supposed to be called like:
ossStrNCopy( pszTarget, pszSource, sizeof(pszTarget) ) ;
(where pszTarget is a char [] array and not a char *). The idea is to avoid buffer overflows that would occur if you were to copy more bytes than the target can hold.
This incorrect use is an excellent example of what was probably clueless cut and paste. Perhaps the thought process was: “I don’t know what this last parameter is for. It’s too much work to read the documentation … I’ll just put something.”
The caller is requesting to have one plus the null terminator of the source string copied into the target. If you are going to do that, why not just call strcpy?
Kuba Ober said
I’m thinking that even in C using the naked pointer types as strings is tragic. The use of standard C library for strings should be IMHO forbidden in production code. In most of my microcontroller C-based projects, I use a custom library that exposes strings as opaque pointers that just happen to point to the beginning of the character array, for the times when you need to index such a beast. Even then I leave the indexing to a macro that typecasts the pointer and optionally does bounds checking. It’s impossible to use the opaque pointer in place of a C-string inadvertently. Of course using naked pointers as strings in C++ is wholly inexcusable. Legacy interfaces notwithstanding.
peeterjoot said
Our codebase is at least 20 years old … more legacy than non-legacy. There’s lots that we do that should be forbidden;)