user908015
user908015

Reputation:

Using strncpy() to copy const char *

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!

Upvotes: 2

Views: 15240

Answers (4)

wildplasser
wildplasser

Reputation: 44250

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

Upvotes: 1

Stephen Canon
Stephen Canon

Reputation: 106317

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.

Upvotes: 3

Kerrek SB
Kerrek SB

Reputation: 477580

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
}

Upvotes: 1

Some programmer dude
Some programmer dude

Reputation: 409472

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.

Upvotes: 5

Related Questions