1

I want to allocate memory to structures in function and assign the values given as parameters to the function. My code:

linkedList.c

FrameNode* createFrameNode(char name[MAX_NAME], int duration, char path[MAX_PATH])
{
    Frame* newFrame = malloc(sizeof(Frame));
    FrameNode* newNode = malloc(sizeof(FrameNode));

    // Copy name
    strcpy(newFrame->name, name);

    // Add duration and path
    newFrame->duration = duration;
    strcpy(newFrame->path, path);

    // Attach new frame to the new node
    newNode->frame = newFrame;
    newNode->next = NULL;

    return newNode;
}

I get an error back:

Exception thrown at 0x00007FFF18C4D215 (ucrtbased.dll) in C_Project.exe: 0xC0000005: Access violation reading location 0xFFFFFFFFFFFFFFFF.

If I try to get any memory related to the structs. Can you tell what is wrong from my code?

The structs in linkedList.h

// 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;
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
Ziv Sion
  • 424
  • 4
  • 14
  • 3
    You haven't allocated memory to the `struct` members. So `strcpy(newFrame->name, name);` will fail. But `newFrame->name = name;` will be OK (although if the source changes so will this data). Alternatively `newFrame->name = strdup(name);` – Weather Vane Jun 21 '22 at 12:26
  • What do you mean by function members? – Ziv Sion Jun 21 '22 at 12:29
  • Typo sorry - corrected. – Weather Vane Jun 21 '22 at 12:29
  • @איתמרשיאון After `Frame* newFrame = malloc(sizeof(Frame));`, `newFrame->name` is uninitialized and just points to some random memory location. You need to initialize it correctly before using it. Likewise for `newFrame->path` . – G.M. Jun 21 '22 at 12:33
  • God bless you!! – Ziv Sion Jun 21 '22 at 12:33
  • Is there a compelling reason for your `struct` to use pointer variables as opposed to simple `char` arrays? (if you are not constrained, it would simplify the code to make that change.) `char* name;` -> `char name[80];` – ryyker Jun 21 '22 at 13:45

2 Answers2

0

The data member name of the structure struct Frame has the pointer type char *

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

After this allocation of memory for an object of the type struct Frame

Frame* newFrame = malloc(sizeof(Frame));

the data member name has an indeterminate value.

So this call of strcpy

// Copy name
strcpy(newFrame->name, name);

invokes undefined behavior.

You need to allocate memory where you are going to copy the string pointed to by the pointer name like for example

// Copy name
newFrame->name = malloc( strlen( name ) + 1 );
strcpy(newFrame->name, name);

The same problem exists for the data member path. That is you need to allocate a character array where you are going to copy the string pointed to by the pointer path.

In general you should check that all memory allocations were successful.

Pay attention to that these parameter declarations char name[MAX_NAME] and char path[MAX_PATH] are adjusted by the compiler to the declarations char *name and char *path. These parameters should be declared with qualifier const because the passed strings are not changed within the function.

FrameNode* createFrameNode(const char name[MAX_NAME], int duration, const char path[MAX_PATH]);
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
0

It is also not uncommon to simplify life by not including pointer members into a struct in the first place. Although they do have their place, if not absolutely necessary, the memory allocation and freeing steps required will add unnecessary complication to the code...

So, unless you are constrained to use pointers, just use char arrays instead.

// Frame struct
typedef struct Frame
{
    char name[80];
    unsigned int duration;
    char path[MAX_PATH_LEN];//or whatever define is provided on your system
} Frame; 
ryyker
  • 22,849
  • 3
  • 43
  • 87