1

Im need to create a dynamically allocated list of x,y points:

typedef struct
{
    uint16_t x;
    uint16_t y;
} point_t;

A series of points will be added to a structure:

typedef struct
{
    uint32_t length;
    uint32_t size;
    point_t *pointlist;
} pointlist_t;

So this code will allocate the first pointer to the pointslist_t structure but returns null when trying to allocate the point_t *.

bool pointInit(pointlist_t *pointlist)
{
    // get storage for the points list
    pointlist = (pointlist_t *)malloc(sizeof(pointlist_t)); 
    if (pointlist == NULL)
    {
        free(pointlist);
        return false;
    }

    // now the list of points
    pointlist->pointlist = (point_t *)malloc(sizeof(point_t)); 
    if (pointlist->pointlist == NULL)
    {
        free(pointlist->pointlist);
        return false;
    }
    pointlist->length = 0;
    pointlist->size = 0;
    return true;
}

In the main, I call the list:

    pointlist_t * mypointlist;
    pointInit(mypointlist);

I cant figure out why. Ideas?

Richard Klosinski
  • 91
  • 1
  • 8
  • 2
    You have forgotten that arguments in C are pass by *value*. That means the value in the call is copied into the functions local argument variable. Modifications to the variable, like assigning to it, will be lost once the life-time of the variable ends, when the function returns. Easy solution: *Return* the pointer instead, and use its `NULL` or not value as a success/failure status. – Some programmer dude Jul 26 '22 at 16:45
  • 1
    Why are you freeing those allocations if they return null? Also why do you think `pointlist->pointlist` is a `point_t *` and not a `pointlist_t *`? – Blindy Jul 26 '22 at 16:45
  • 1
    Side note: 1. Casting results of `malloc()` family is [considered as a bad practice](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). 2. Freeing what is confirmed to be `NULL` by `if` statement is meaningless. – MikeCAT Jul 26 '22 at 16:45
  • And as mentioned, in `if (pointlist->pointlist == NULL)` passing `pointlist->pointlist` to `free` is pretty useless. But you're missing a `free(pointlist)` there, leading to a memory leak. – Some programmer dude Jul 26 '22 at 16:47
  • 1. You need to pass `pointlist_t **` in order for `pointInit` to update `mypointlist` from `main`. 2. no need to free a pointer if it is NULL. 3. pointlist->pointlist should be malloced with a memory corelated to the length/size, and if it's 0 it can be simply NULL. – wohlstad Jul 26 '22 at 16:47

0 Answers0