1

I've been struggling to create a new node, that has a pointer to a node of different type in it, and a pointer to the next node. Below are my two structs:

// Frame struct
typedef struct Frame
{
    char* name;
    unsigned int duration;
    char* path;
} Frame;

// Link (node) struct
typedef struct FrameNode
{
    Frame* frame;
    struct FrameNode* next;
} FrameNode;

And my attempt to create a new node of type FrameNode:

/**
This function creates a new frame.
input: name - the name of the frame we want to create, duration - the duration of the frame we want to create, path - the path of the frame we want to create
output: newFrame - a new frame, that will be added to the end of the list of frames
*/

FrameNode* createFrame(char* name, unsigned int duration, char* path) {
    FrameNode* newFrame = (FrameNode*)malloc(sizeof(FrameNode));
    newFrame->frame = (Frame*)malloc(sizeof(Frame)); // create memory for the Frame* inside of FrameNode*

    strcpy(newFrame->frame->name, name);
    newFrame->frame->duration = duration;
    strcpy(newFrame->frame->path, path);

    return (newFrame);
}

I think the problem is related somehow to strcpy, but I'm not entirely sure. I've used it in the past with no issues at all, so that's strange. Thanks :)

  • 2
    These statementfs strcpy(newFrame->frame->name, name); strcpy(newFrame->frame->path, path); invoke undefined behavior because the pointers name and path have indeterminate values. – Vlad from Moscow Jun 06 '21 at 19:35
  • Unrelated, I'm not convinced the `frame` member of `FrameNode` needs to be dynamic. Related, if you're using a POSIX compliant implementation, `strdup` will save you some steps regarding copying those strings. – WhozCraig Jun 06 '21 at 19:36
  • @VladfromMoscow tried changing it to this: strncpy(newFrame->frame->name, name, nameSize); newFrame->frame->duration = duration; strncpy(newFrame->frame->path, path, pathSize); to no avail. nameSize and pathSize are the results of strlen(name) and strlen(path), I forgot to put them as parameters in the post. – RaphDaPingu Jun 06 '21 at 19:47
  • @RaphDaPingu I am sorry. I meant newFrame->frame->name and newFrame->frame->path . You need to allocate memory dynamically for these pointers. – Vlad from Moscow Jun 06 '21 at 19:52

1 Answers1

1

You are not allocating memory for the strings, which makes you have undefined behavior in your code. If you have strdup(), here's one way of doing it:

/**
This function creates a new frame.
input: name - the name of the frame we want to create, duration - the duration of the frame we want to create, path - the path of the frame we want to create
output: newFrame - a new frame, that will be added to the end of the list of frames
*/

FrameNode* createFrame(char* name, unsigned int duration, char* path) {
    FrameNode* newFrame = malloc(sizeof *newFrame);
    newFrame->frame = malloc(sizeof *newFrame->frame);

    newFrame->frame->name = strdup(name);
    newFrame->frame->duration = duration;
    newFrame->frame->path = strdup(path);

    return newFrame;
}

I rewrote the allocations since I think you should not cast the return value of malloc(), and I believe that style of sizeof-usage is better than repeating the type name. Also, less parentheses. :)

Note that memory allocation can fail, so for "production quality" this would at least need to check that.

unwind
  • 391,730
  • 64
  • 469
  • 606