Using strncpy() to copy const char *

13,002

Solution 1

Well, you allocate the structure, but not the string inside the structure. You need to do that before you copy to it. Even when you do, you will probably overwrite unallocated memory when you attempt to set the string terminator.

And, due to a hight intake ow wine, I just noticed you actually only copy one character, but it's still undefined behavior.

Solution 2

Let's take this one step at a time:

struct test *t1 = malloc(sizeof(struct test));

this allocates space for a struct test; enough space for the pointer name, but not any memory for the pointer to point to. At a minimum, you'll want to do the following:

t1->name = malloc(strlen(s) + 1);

Having done that, you can proceed to copy the string. However, you already computed the length of the string once to allocate the memory; there's no sense in doing it again implicitly by calling strncpy. Instead, do the following:

const size_t len = strlen(s) + 1;  // +1 accounts for terminating NUL
t1->name = malloc(len);
memcpy(t1->name, s, len);

In general, try to use this basic pattern; compute the length of strings once when they come into your code, but then use explicit-sized memory buffers and the mem* operations instead of implicit-length strings with str* operations. It is at least as safe (and often safer) and more efficient if done properly.

You might use strncpy if t1->name was a fixed-size array instead (though many people prefer to use strlcpy). That would look like the following:

struct test { char name[MAXSIZE]; };
struct test *t1 = malloc(sizeof *t1);
strncpy(t1->name, s, MAXSIZE - 1);
t1->name[MAXSIZE-1] = 0; // force NUL-termination

Note that the size argument to strncpy should always be the size of the destination, not the source, to avoid writing outside the bounds of the destination buffer.

Solution 3

Without any attempt at completeness or educational direction, here's a version of your code that should work. You can play "spot the difference" and search for an explanation for each one separately on this site.

int main()
{ 
    const char s[] = "how";                 // s is an array, const char[4]

    struct test{ char name[NAMESIZE]; };    // test::name is an array

    struct test * t1 = malloc(sizeof *t1);  // DRY

    strncpy(t1->name, s, NAMESIZE);         // size of the destination
    t1->name[NAMESIZE - 1] = '\0';          // because strncpy is evil

    printf("%s\n", t1->name);

    free(t1);                               // clean up
}

Solution 4

strncpy() is always wrong

  • if the result is too long, the target string will not be nul-terminated
  • if the target is too long (the third argument) , the trailing end will be completely padded with NULs. This will waste a lot of cycles if you have large buffers and short strings.

Instead, you cound use memcpy() or strcpy, (or in your case even strdup() )

int main()
{
const char *s = "how";

struct test {
    char *name;
    };
struct test *t1
size_t len;

t1 = malloc(sizeof *t1);

#if USE_STRDUP

  t1->name = strdup(s);

#else

  len = strlen(s);
  t1->name = malloc (1+len);
  memcpy(t1->name, s, len);
  t1->name[len] = '\0';

#endif    

printf("%s\n", t1->name);

return 0;
}
Share:
13,002
isal
Author by

isal

Updated on June 14, 2022

Comments

  • isal
    isal almost 2 years

    I'm very new to C, I'm getting stuck using the strncpy function.\

    Here's an example of what I'm working with:

    int main()
    {
    
    const char *s = "how";
    
    struct test {
        char *name;
    };
    
    struct test *t1 = malloc(sizeof(struct test));
    
    strncpy(t1->name, s, sizeof(*s));
    t1->name[NAMESIZE] = '\0';
    
    printf("%s\n", t1->name);
    
    }
    

    I have a const char *, I need to set the "name" value of test to the const char. I'm having a really tough time figuring this out. Is this even the correct approach?

    Thank you very much!