0

I'm having some problems with this function:

link_t* createFrame(char name[], int duration, char path[]){
    link_t* newFrame = (link_t*)malloc(sizeof(link_t));
    newFrame->frame= (frame_t*)malloc(sizeof(frame_t));
    newFrame->frame->duration = duration;
    newFrame->frame->name = (char*)malloc((MAX_NAME_SIZE + 1) * sizeof(char));
    newFrame->frame->name[MAX_NAME_SIZE] = 0; //Manually add null termination
    strncpy(newFrame->frame->name, name, MAX_NAME_SIZE);
    newFrame->frame->path = (char*)malloc((MAX_NAME_SIZE + 1) * sizeof(char));
    newFrame->frame->path[MAX_PATH_SIZE] = 0;

    strncpy(newFrame->frame->path, path, MAX_PATH_SIZE);

    newFrame->next = NULL;
    return newFrame;
}

And here's 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;

The problem is when I use the function in a loop, the first time it works but the next time it doesn't. I tried to debug the code step by step and I found out that the problem is in the second run of the loop, in this line of code:

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

Maybe I need to free something in the loop?

Barmar
  • 741,623
  • 53
  • 500
  • 612
Saga
  • 67
  • 1
  • 8
  • Forgetting to free something won't cause a failure. It just causes a memory leak, so it's only a problem if you create so many objects that you run out of memory. – Barmar Jun 09 '17 at 21:33
  • 2
    [don't cast malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Barmar Jun 09 '17 at 21:35
  • 3
    `newFrame->frame->path = (char*)malloc((MAX_NAME_SIZE + 1) * sizeof(char));` --> `newFrame->frame->path = (char*)malloc((MAX_PATH_SIZE + 1) * sizeof(char));` – BLUEPIXY Jun 09 '17 at 21:37
  • @Barmar Same error. – Saga Jun 09 '17 at 21:38
  • @Saga I didn't post an answer, what are you talking about? Did you fix the typo that BLUEPIXY pointed out? – Barmar Jun 09 '17 at 21:39
  • @Saga Casting malloc has nothing to do with this error, it's just a general recommendation I post whenever I see code like that. – Barmar Jun 09 '17 at 21:39
  • @Barmar Oh and when it crush I'm getting in visual studio this function:__forceinline void * __cdecl _heap_alloc (size_t size) – Saga Jun 09 '17 at 21:45
  • @Saga You're probably writing outside array bounds because you're using the wrong size when you call `malloc()` for `newFrame->frame->path`. That causes undefined behavior. – Barmar Jun 09 '17 at 21:50
  • @Barmar You're right probably because I tried to delete all the strings from the function(and the malloc for the string) and it worked. But how do I change this code to work with strings as well? – Saga Jun 09 '17 at 22:04
  • 1
    Just fix the typo that BLUEPIXY pointed out above. `MAX_NAME_SIZE` should be `MAX_PATH_SIZE`. – Barmar Jun 09 '17 at 22:06
  • @BLUEPIXY Thanks, it worked :) – Saga Jun 09 '17 at 22:09
  • 1
    You can also use `calloc` instead of `malloc` on both `newFrame->frame->name` (and `...->path`) and avoid the specific *nul-termination* (up to you). – David C. Rankin Jun 09 '17 at 22:44

0 Answers0