3

I have the following code in C now

int length = 50
char *target_str = (char*) malloc(length);
char *source_str = read_string_from_somewhere() // read a string from somewhere
                                                //    with length, say 20
memcpy(target_str, source_str, length);

The scenario is that target_str is initialized with 50 bytes. source_str is a string of length 20.

If I want to copy the source_str to target_str i use memcpy() as above with length 50, which is the size of target_str. The reason I use length in memcpy is that, the source_str can have a max value of length but is usually less than that (in the above example its 20).

Now, if I want to copy till length of source_str based on its terminating character ('\0'), even if memcpy length is more than the index of terminating character, is the above code a right way to do it? or is there an alternative suggestion.

Thanks for any help.

abisheksampath
  • 376
  • 8
  • 23
  • 2
    Why not `strlen(source_str)`, then you can alloc the exact size? Also, you don't need to cast the result of `malloc`. – ggorlen Apr 24 '19 at 21:05
  • 1
    Add +1 for null – Ed Heal Apr 24 '19 at 21:06
  • Possible duplicate of [Access element beyond the end of an array in C](https://stackoverflow.com/questions/1021021/access-element-beyond-the-end-of-an-array-in-c) – Mgetz Apr 24 '19 at 21:20

5 Answers5

3

The scenario is that target_str is initialized with 50 bytes. source_str is a string of length 20.

If I want to copy the source_str to target_str i use memcpy() as above with length 50, which is the size of target_str.

currently you ask for memcpy to read 30 characters after the end of the source string because it does not care of a possible null terminator on the source, this is an undefined behavior

because you copy a string you can use strcpy rather than memcpy

but the problem of size can be reversed, I mean the target can be smaller than the source, and without protection you will have again a undefined behavior

so you can use strncpy giving the length of the target, just take care of the necessity to add a final null character in case the target is smaller than the source :

int length = 50
char *target_str = (char*) malloc(length);
char *source_str = read_string_from_somewhere(); // length unknown

strncpy(target_str, source_str, length - 1); // -1 to let place for \0
target_str[length - 1] = 0; // force the presence of a null character at end in case
bruno
  • 32,421
  • 7
  • 25
  • 37
2

If I want to copy the source_str to target_str i use memcpy() as above with length 50, which is the size of target_str. The reason I use length in memcpy is that, the source_str can have a max value of length but is usually less than that (in the above example its 20).

It is crucially important to distinguish between

  • the size of the array to which source_str points, and
  • the length of the string, if any, to which source_str points (+/- the terminator).

If source_str is certain to point to an array of length 50 or more then the memcpy() approach you present is ok. If not, then it produces undefined behavior when source_str in fact points to a shorter array. Any result within the power of your C implementation may occur.

If source_str is certain to point to a (properly-terminated) C string of no more than length - 1 characters, and if it is its string value that you want to copy, then strcpy() is more natural than memcpy(). It will copy all the string contents, up to and including the terminator. This presents no problem when source_str points to an array shorter than length, so long as it contains a string terminator.

If neither of those cases is certain to hold, then it's not clear what you want to do. The strncpy() function may cover some of those cases, but it does not cover all of them.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
1

memcpy is used to copy fixed blocks of memory, so if you want to copy something shorter that is terminated by '\n' you don't want to use memcpy.

There is other functions like strncpy or strlcpy that do similar things. Best to check what the implementations do. I removed the optimized versions from the original source code for the sake of readability.

This is an example memcpy implementation: https://git.musl-libc.org/cgit/musl/tree/src/string/memcpy.c

void *memcpy(void *restrict dest, const void *restrict src, size_t n)
{
    unsigned char *d = dest;
    const unsigned char *s = src;
    for (; n; n--) *d++ = *s++;
    return dest;
}

It's clear that here, both pieces of memory are visited for n times. regardless of the size of source or destination string, which causes copying of memory past your string if it was shorter. Which is bad and can cause various unwanted behavior.

this is strlcpy from: https://git.musl-libc.org/cgit/musl/tree/src/string/strlcpy.c

size_t strlcpy(char *d, const char *s, size_t n)
{
    char *d0 = d;
    size_t *wd;

    if (!n--) goto finish;
    for (; n && (*d=*s); n--, s++, d++);
    *d = 0;
finish:
    return d-d0 + strlen(s);
}

The trick here is that n && (*d = 0) evaluates to false and will break the looping condition and exit early.

Hence this gives you the wanted behaviour.

Alexander Oh
  • 24,223
  • 14
  • 73
  • 76
1

Now, if I want to copy till length of source_str based on its terminating character ('\0'), even if memcpy length is more than the index of terminating character, is the above code a right way to do it?

No; you'd be copying the entire content of source_str, even past the null-terminator if it occurs before the end of the allocated space for the string it is pointing to.

If your concern is minimizing the auxiliary space used by your program, what you could do is use strlen to determine the length of source_str, and allocate target_str based on that. Also, strcpy is similar to memcpy but is specifically intended for null-terminated strings (observe that it has no "size" or "length" parameter):

char *target_str = NULL;
char *source_str = read_string_from_somewhere();
size_t len = strlen(source_str);

target_str = malloc(len + 1);

strcpy(target_str, source_str);

// ...

free(target_str);
target_str = NULL;
Govind Parmar
  • 20,656
  • 7
  • 53
  • 85
0

Use strlen to determine the exact size of source_string and allocate accordingly, remembering to add an extra byte for the null terminator. Here's a full example:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void) {
    char *source_str = "string_read_from_somewhere";

    int len = strlen(source_str);
    char *target_str = malloc(len + 1);

    if (!target_str) {
        fprintf(stderr, "%s:%d: malloc failed", __FILE__, __LINE__);
        return 1;
    }

    memcpy(target_str, source_str, len + 1);

    puts(target_str);
    free(target_str);

    return 0;
}

Also, there's no need to cast the result of malloc. Don't forget to free the allocated memory.

As mentioned in the comments, you probably want to restrict the size of the malloced string to a sensible amount.

ggorlen
  • 44,755
  • 7
  • 76
  • 106
  • I always feel a disturbance in the force when I see a naked `strlen()` result put into a `malloc()` or `memcpy()`. This is an exploit waiting to happen :) But it does indeed answer the question. – Michael Dorgan Apr 24 '19 at 22:43