strcpy... want to replace with strcpy_mine which will strncpy and null terminate

Question

the clue is in the title but basically I've inherited some code which has 800+ instances of strcpy. I want to write a new function and then to replace strcpy with strcpy_mine.

So I'm trying to work out what parameter list strcpy_mine will have.

I tried:

void strcpy_mine( char* pTarget, const char* const pCopyMe )
{
  const unsigned int lenAlwaysFour = sizeof(pCopyMe ); //:(
  strncpy( pTarget, pCopyMe, lenAlwaysFour );

  //add extra terminator in case of overrun
  pTarget[lenAlwaysFour] = 0;
}

but the sizeof is always 4 pCopyMe is a pointer

what I don't want to do is replace

strcpy (buf, pCopyMe);

with

strncpy (buf, pCopyMe, sizeof(pCopyMe)); buf[sizeof(pCopyMe)] = 0;

any ideas? (strcpy_l isn't available)

cheers

Answer 1

sizeof() returns the size of the type - in this case const char* const which will be 4 on 32-bit machines.

I think you think you want strlen(). But that isn't the right way to use strncpy functions. You need the size of the output buffer for strncpy.

To fix this you need to examine the code at each call site, and work out the size of the output buffer, and pass that as an argument to strcpy_mine. If the call-site for strcpy (or strcpy_mine) doesn't know the size of the output buffer, you need to search backwards in the code for the location that allocates the buffer, and pass the size all the way down to the strcpy site.

Basically you can't write a drop in replacement for strcpy that takes the same arguments and hope to avoid the problems that produced strncpy in the first place (and better replacements beyond that). You can create a function that takes the same arguments as strncpy, but ensures the result is null-terminated - look at the implementation of OpenBSD's strlcpy() function. But the first step has to be to change the calling sites to pass on knowledge of the output buffer size.

Answer 2

You could use the same parameter list as strncpy for your strcpy_mine, but write it so that it always null terminates the result. Shouldn't be very hard to do.

One challenge, however, is that some of your existing code that calls strcpy() may not know the size of the buffer, either.

Answer 3

Have you considered using sprintf ?

such as

sprintf(pTarget, "%s\0", pCopyMe);

Answer 4

Also You can use macroses for avoid multiple editings. Or automate editing via some script.

Answer 5

You definitely need to pass in the size of the destination buffer as a parameter, as other people have said above.

This is kind of off-topic, but I just want to point out that, after you use strncpy(), you need to set to null the last character of the buffer, which has index 1 less than the length (not the length of the buffer):

strncpy (buf, pCopyMe, buflen); buf[buflen - 1] = '\0';

Or alternatively, you can use strncat() on an empty string, passing it a length that is 1 less, and it will guarantee to null-terminate your string:

buf[0] = '\0'; strncat (buf, pCopyMe, buflen - 1);

Answer 6

Douglas Leeder has it right. There is a limit to the usefulness of replacing strcpy unless you are willing to do the grunt work of passing in a good, sane buffer length at each and every instance. That's a lot of work!

The good news is, it is worth it! Back a few years ago, I came in on several C++ projects that were late, buggy, and unreliable. By declaring strcpy and strlen forbidden, and taking 2-3 days out of the project to replace them with custom strncpy/strnlen, in all these projects we suddenly could run for days instead of hours. We also saw a lot of truncated strings come out on screen displays and log files. That gave us the clues needed to track down the truncation issues, formerly crashing issues.

If you do not want to do this, you can get a much smaller benefit by simply checking both pointer parameters for NULL, and limiting the maximum size of a string copy and logging all times that the boundary is reached. Do not do a strlen of either parameter, as strlen will happily crash on you if the string is not properly null terminated.

Nowadays, new projects use good string objects, but there is a lot of legacy code out there that does not.

Answer 7

Depending on how the call-sites look like, often majority of cases can be handled by a simple template:

#include <string.h>

template <int bufferSize>
void strcpy_mine( char (&pTarget)[bufferSize], const char* const pCopyMe )
{
  strncpy( pTarget, pCopyMe, bufferSize-1 );

  //add extra terminator in case of overrun
  pTarget[bufferSize-1] = 0;
}

int main()
{
  char buf[128];
  strcpy_mine(buf,"Testing");
  return 0;
}

If you are using Microsoft Visual Studio 2005 or newer, see Secure Template Overloads for a Microsoft implementation.

Answer 8

Slightly peripheral perhaps, but since noone mentioned it and it's flaunted in the title: you can't (legally) write a global function called strcpy_mine().

The "namespace" of functions whose names start with str is reserved for the standard library. See, for instance, the accepted answer to this question.