why does strcpy copies more character to the variable than it is supposed to?

Question

I am currently working on with lots of strcpy' and calloc's. And then I heard that strncpy is safer to use. So what I did was create a function that will handle strcpy.. It is shown below.

void safeStrncpy(char * dest, char * src){
    //copy string 
    if(sizeof(dest) >= strlen(src) + 1){
        strncpy(dest, src, strlen(src));
    }else{
        if(realloc(dest, (strlen(src)) + 1) == NULL){
            printf("error");    
        }else{
            strncpy(dest, src, strlen(src));
        }
    }
}

You'll notice that I used sizeof(dest) . What I really want to do with that part is get the size of the memory allocated to dest so I'll know when to use realloc. But then I learned that you can't get the size of memory allocated to dest so I think of a workaround.

char * l = calloc(10,sizeof(char));
printf("%s", strcpy(l,"asdfghjkldfghasdfghjkl"));

The code shown above allocates 10 items to the pointer l. I thought that If I did that and copied more characters than what is allocated, it will just copy what fits to the size.. I was expecting that the value of l would be "asdfghjkld". But, to my surprise, it didn't. It copied the whole string which is this "asdfghjkldfghasdfghjkl".

Now, why use calloc if it will just be overridden? what happens in the background during char pointer declaration, memory allocation and strcpy? does c automatically reallocates the memory? Do I need to worry of this behavior, security issues maybe?

`strcpy` does what it says, it copies a null terminated string to where you tell it. It is up to you to make sure you've allocated enough space. `sizeof(dest)` is the size of a pointer, not the length of the string. — Retired Ninja, Aug 31 '14 at 05:11
Also, your code is wrong above in the `realloc` case -- realloc will return a new pointer, but you'd lose that pointer on function exit. You'd need to pass in the pointer-to-pointer to dest (char **). — Joe, Aug 31 '14 at 16:07

score 1 · Accepted Answer · answered Aug 31 '14 at 06:09

You asked:

why does strcpy copies more character to the variable than it is supposed to?

The documentation of strcpy:

Copies the C string pointed by source into the array pointed by destination, including the terminating null character (and stopping at that point).

To avoid overflows, the size of the array pointed by destination shall be long enough to contain the same C string as source (including the terminating null character), and should not overlap in memory with source.

It doesn't specify how a program should behave should the destination be not long enough to contain the source. Most likely, your implementation simply writes over memory that is out of bounds, which would lead to undefined behavior.

Here's a possible solution.

Create a struct that hold the char* and a size.

typedef struct _MyString
{
   char* data;
   size_t size;
} MyString;

Use it in the safeStrncpy

void safeStrncpy(MyString* dest, char * src){
  if(dest->size < strlen(src) + 1){
    dest->size = strlen(src)+1;
    dest->data = realloc(dest->data, dest->size);
  }
  strcpy(dest->data, src);
}

Just to complete the answer : Generally speaking with c when reading the mans, try to search every occurence of "undefined behavior" and learn them before using the function :) — Logar, Aug 31 '14 at 06:15

score 0 · Answer 2 · answered Aug 31 '14 at 06:30

The code you show misuses strncpy(). It also misuses sizeof(). And errors, if reported by printing a message, should be printed to standard error, aka stderr (and not standard output). Finally, if you reallocate the memory, the calling code does not know this, which leads to problems when the realloc() moves the data around memory. The current code is:

void safeStrncpy(char * dest, char * src){
    //copy string 
    if(sizeof(dest) >= strlen(src) + 1){
        strncpy(dest, src, strlen(src));
    }else{
        if(realloc(dest, (strlen(src)) + 1) == NULL){
            printf("error");    
        }else{
            strncpy(dest, src, strlen(src));
        }
    }
}

sizeof(dest) is the size of the pointer (probably 4 or 8 bytes). You have to know how big the destination string is, so you need to pass that in as an argument.
strncpy(dest, src, strlen(src)) guarantees that the string is not null terminated. It tells strncpy() to copy only as many non-null characters as there are in the string.

I suggest:

void safeStrncpy(char **p_buffer, size_t *p_bufsiz, const char *src)
{
    size_t newlen = strlen(src) + 1;
    if (*p_bufsiz < newlen)
    {
        char *n_buffer = malloc(newlen);
        if (n_buffer == 0)
        {
            fprintf(stderr, "Failed to allocate %zu bytes memory\n", newlen);
            return;
        }
        *p_buffer = n_buffer;
        *p_bufsiz = newlen;
    }
    memmove(*p_buffer, src, newlen);
}

Since I know exactly how long the string is, there's no need to use strcpy() which must check each character to see if it is null; using memmove() — or memcpy() if you prefer, as it is safe this time — is likely to be quicker.

Sample calling sequence:

char *buffer = 0;
size_t bufsiz = 0;

char line[4096];

while (fgets(line, sizeof(line), stdin) != 0)
{
    safeStrncpy(&buffer, &bufsiz, line);
    …use buffer (which is of size bufsiz)…
}

free(buffer);  // All done

Note that you should document that *p_buffer and src must not overlap at all.

why does strcpy copies more character to the variable than it is supposed to?

2 Answers2