Working with linked list + struct C

Question

Here is the linked list and the struct:

#define MAX_PATH_SIZE (256)
#define MAX_NAME_SIZE (50)
struct Frame
{
    char *name;
    unsigned int duration;
    char *path;  // may change to FILE*
};

typedef struct Frame frame_t;
struct Link
{
    frame_t *frame;
    struct Link *next;
};

typedef struct Link link_t;

And here's my function:

link_t* createFrame(char name[], int duration, char path[]){
frame_t * temp = (frame_t*)malloc(sizeof(frame_t));
temp->duration = duration;

strncpy(temp->name, name,MAX_NAME_SIZE);
strncpy(temp->path, path,MAX_PATH_SIZE);
link_t* newFrame = (link_t*)malloc(sizeof(link_t));

newFrame->frame = temp;
return newFrame;
}

The problem is that the function stop working in the line "strncpy(temp->name)..", the weird thing is that the temp->duration is working but it doesn't work with strings. Error: "Unhandled exception at 0x0F744645 (msvcr120d.dll)"

Possible duplicate of ['strcpy' with 'malloc'?](https://stackoverflow.com/questions/5354933/strcpy-with-malloc) — jpw, Jun 08 '17 at 16:30

unalignedmemoryaccess · Answer 1 · 2017-06-08T18:09:01.400

2

You didn't allocate memory for your name, now they point to unknown location and is undefined behaviour.

temp->name = malloc((MAX_NAME_SIZE + 1) * sizeof(*temp->name));
temp->path = malloc((MAX_PATH_SIZE + 1) * sizeof(*temp->path));
temp->name[MAX_NAME_SIZE] = 0; //Manually add null termination
temp->name[MAX_PATH_SIZE] = 0; //Manually add null termination
strncpy(temp->name, name,MAX_NAME_SIZE);

Now, memory is allocated for name and path and you are able to copy data for name and path.

Or, if you want, you can define your structure like this:

struct Frame {
    char name[MAX_NAME_SIZE + 1];
    unsigned int duration;
    char path[MAX_PATH_SIZE + 1];
};

Then you won't need to call malloc for name and path separatelly as memory will be allocated on first malloc for structure already.

edited Jun 08 '17 at 18:09

answered Jun 08 '17 at 16:27

unalignedmemoryaccess

7,246
2
25
40

1

Note With `strncpy(temp->name, name,MAX_NAME_SIZE);`, `temp->name, name` may lack a _null character_. – chux - Reinstate Monica Jun 08 '17 at 17:53
The allocation is OK, yet `name[MAX_NAME_SIZE];` remains uninitialized with `strncpy(temp->name, name,MAX_NAME_SIZE);`. – chux - Reinstate Monica Jun 08 '17 at 17:59
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/146195/discussion-between-chux-and-tilz0r). – chux - Reinstate Monica Jun 08 '17 at 18:03
I tried the first way and in the first run(in loop) it works, but in the second it doesn't work, do I need to free something in the loop? – Saga Jun 09 '17 at 21:07

score 0 · Answer 2 · answered Jun 08 '17 at 16:30

0

You need to allocate memory to store strings with strncpy(), in your malloc you allocated only enough to store sizeof(struct Frame) bytes.

You may want to try this instead of strncpy...

temp->name = strndup(name, MAX_NAME_SIZE);
temp->path = strndup(path,MAX_PATH_SIZE);

...if you are insisting on restricting the max size of strings.

answered Jun 08 '17 at 16:30

Sokre

114
6

Note that `strndup()` is not a standard C library function. The implementations I have seen the 2nd argument is 1 less than the max _size_. It is more like a max string length. – chux - Reinstate Monica Jun 08 '17 at 17:58
What does it mean "not standard"? Which standard? This one - http://man7.org/linux/man-pages/man3/strndup.3p.html ? And that there are implementation deviating from the standard, is natural. Talking of standard, "the man" seems to be using Microsoft C. He might not have strndup() at all. – Sokre Jun 09 '17 at 06:43
"Which standard?" --> "standard C library" is referring to the library of functions specified in the C11 spec. [ref](https://stackoverflow.com/q/81656/2410359). Deviating from the _C_ standard makes the implementation non-compliant. As `strndup()` is not part of the standard C library function, its signature and functionality may vary. Using `strndup()` to solve OP's issues is an OK idea yet it lacks portability. – chux - Reinstate Monica Jun 09 '17 at 13:23

score 0 · Answer 3 · answered Jun 08 '17 at 17:30

0

Try this :

#define MAX_PATH_SIZE (256)
#define MAX_NAME_SIZE (50)

struct Frame
{
    char name[MAX_NAME_SIZE];
    unsigned int duration;
    char path[MAX_PATH_SIZE];  // may change to FILE*
};

typedef struct Frame frame_t;
struct Link
{
    frame_t *frame;
    struct Link *next;
};

typedef struct Link link_t;

link_t* createFrame(char name[MAX_NAME_SIZE], int duration, char path[MAX_PATH_SIZE]){
    frame_t * temp = (frame_t*)malloc(sizeof(frame_t));
    temp->duration = duration;

    strncpy(temp->name, name, MAX_NAME_SIZE);
    strncpy(temp->path, path, MAX_PATH_SIZE);
    link_t* newFrame = (link_t*)malloc(sizeof(link_t));

    newFrame->frame = temp;
    return newFrame;
}

answered Jun 08 '17 at 17:30

Adam

33
4

2

What if string is long `MAX_NAME_SIZE` characters and you copy it to `temp->name`. You have undefined behaviour because of *null* termination. – unalignedmemoryaccess Jun 08 '17 at 17:58
`strncpy(temp->name, name, MAX_NAME_SIZE);` does not certainly _null character_ terminate `temp->name`. Could append `temp->name[MAX_NAME_SIZE - 1] = 0;` to insure proper string termination. – chux - Reinstate Monica Jun 08 '17 at 18:17

Working with linked list + struct C

3 Answers3