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)

Jenifferjenilee

Answer 1

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.

Headwaiter

cheers! this is almost what I'm after, but it still forces me to put the size of the buffer in chevrons though char buf[200]; strcpy_mine<200>(buf, pString); – Jenifferjenilee

strCpyMine<sizeof(buf)>(buf, pString); – Jenifferjenilee

Obsolete compiler I think, I'm using VC6; see support.microsoft.com/kb/165803 – Jenifferjenilee

I was doing something wrong. The template needs to be slightly different. Fixing now. – Headwaiter

Answer 2

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.

Arceliaarceneaux

+1 for strlcpy. In writing my own function similar to this I also pass an enum {AllOrNothing, TruncateOkay} to the function to make it handle the overflow cases. – Bianca

Answer 3

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.

Buschi

I second this. You'll need to add another parameter for the size of the output buffer. The strcpy method is a classic source of buffer overflow errors. Microsoft has even deprecated this function in favor of something like strncpy. – Macegan

Answer 4

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.

Afrikander

Answer 5

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.

Titan

Answer 6

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

Oralee

In case you care, macro is singular, macros is plural. Macroses is not a word. I'm guessing English in not your native language. I only trying to help you out. – Boschvark

Answer 7

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);

Sayres

if u are anyway at it why not: strncpy(buf, pCopyMe, buflen)[buflen-1]='\0'; :-) – Forejudge