Using strncpy on struct

Question

Let's say I have this student struct defined:

struct student {
    char *name;
};
typedef struct student Student

Now I have the following function:

void add_student(const char *student_name) {

    // create new student
    Student *new_s;
    new_s = malloc(sizeof(Student));

    strncpy(new_s->name, student_name, sizeof(new_s->name) - 1)
}

I want to add the student_name to the name of the new student struct. However because const char and char are different I have to use strncpy.

I tried it this way, however I get a segmentation fault, what's wrong?

Instead of `strncpy` you may want to take a look at `strdup`. It sounds very close to what you try to archive here. — Florian Zwoch, Feb 14 '16 at 17:49
**NEVER use `strncpy`** it is never the right tool for the job. — chqrlie, Feb 14 '16 at 18:35

score 5 · Accepted Answer · answered Feb 14 '16 at 17:21

5

You are only allocating memory for the structure new_s in this line

new_s = malloc(sizeof(Student));

This includes the variable char* name, which is a pointer to a char. Although, you also need memory to which this pointer will point at.

So, you need to allocate memory for the character pointer name inside the structure.

// create new student
Student *new_s;
new_s = malloc(sizeof(Student));

new_s->name = malloc(100); //assuming you need to store a string of len 100

answered Feb 14 '16 at 17:21

Haris

12,120
6
43
70

so after allocating new_s->name, i can use strncpy as such? strncpy(new_group->name, group_name, ) – bakedturtle Feb 14 '16 at 17:32
@bakedturtle, The only problem was that you did not allocate the memory. Otherwise your code should work after you add that line. why don't you run and check. – Haris Feb 14 '16 at 17:34
Hmmm - looks like off-by-1 to me. "`...100);` --> //assuming you need to store a string of len 100". The length of a string typically does not include the null character, so suggest "to store a string of length of up to 99". – chux - Reinstate Monica Feb 14 '16 at 20:27

score 2 · Answer 2 · answered Feb 14 '16 at 18:32

As Johan Wentholt correctly outlined in his answer, you must allocate memory for both the Student structure and the string its member name points to, but you must return the new structure so the caller can do something with it:

Student *add_student(const char *student_name) {
    Student *new_s = malloc(sizeof(Student));
    if (new_s) {
        new_s->name = strdup(student_name);
    }
    return new_s;
}

You code invokes undefined behavior because you did not allocate memory for the string, worse even, you left the name member uninitialized (malloc does not initialize the memory it returns).

Furthermore, you should not use strncpy. It is not some safe version of strcpy, it is a very error prone function, whose semantics are poorly understood by most programmers. NEVER use this function. If you see it used, there are chances you are either in front of a bug or there is a better method to replace it.

For completeness, your code:

strncpy(new_s->name, student_name, sizeof(new_s->name) - 1);

Would attempt to copy at most sizeof(char*)-1 characters from student_name into the array pointer to by new_s->name.
If the student_name is longer, the destination will not be null terminated,
If it is shorter, the destination will be padded with null bytes upto the given size.
Here the destination pointer is uninitialized and the size information is bogus anyway: you really want to copy all characters in the string plus the null terminator, which is exactly what strcpy does. But you need to allocate enough memory for that. You could use:
```
new_s->data = malloc(strlen(student_name) + 1);
strcpy(new_s->data, student_name);
```

The Posix function strdup() does both operations in one call:

    new_s->data = strdup(student_name);

This is true, but i was using the example provided by bakedturtle. — 3limin4t0r, Feb 15 '16 at 18:51

3limin4t0r · Answer 3 · 2016-02-14T18:19:37.440

Haris gave a good solution. But like Florian Zwoch said in the comments you can also use strdup like so:

void add_student(const char *student_name)
{
    Student *new_s = malloc(sizeof(Student));
    new_s->name = strdup(student_name);
}

Keep in mind that you have to free new_s->name and than free new_s.

You should also check the return value of malloc and strdup for NULL value. Since it returns NULL if insufficient memory is available.

As side note, you can shorten up the struct and typedef to one statement like so:

typedef struct student {
    char *name;
} Student;

Using strncpy on struct

3 Answers3