Storing data into a dynamic array with structs in C

Question

I am having issues storing data from a file into my dynamic array. I am aware that what I have now is incorrect but it is just there for the moment. I have a file which on the first line contains the amount of lines of data essentially. The following lines have two integers side by side to represent an ordered pair. I want to store those two integers into a struct, point, that symbolizes an ordered pair. Also, the there is an array with such a struct that is inside of another struct, list , which contains the size of the array, or the amount of data currently stored in the array and a capacity which is the total amount of space in the array.

I want to store the two integers into variables of type int and then store them into a point inside of my array that is in my list struct. I am getting very confused having two structs and am unsure if this is the correct approach. Any feedback would be welcomed.

#include <stdio.h>
#include <math.h>
#include <stdlib.h>

typedef struct
{
    int x;
    int y;
} point;

typedef struct
{
    int size;
    int capacity;
    point *A;
} list;

// Compute the polar angle in radians formed
// by the line segment that runs from p0 to p
double polarAngle(point p, point p0)
{
    return atan2(p.y - p0.y, p.x - p0.x);
}

// Determine the turn direction around the corner
// formed by the points a, b, and c. Return a
// positive number for a left turn and negative
// for a right turn.
double direction(point a, point b, point c)
{
    return (b.x - a.x)*(c.y - a.y) - (c.x - a.x)*(b.y - a.y);
}

int whereSmallest(point A[], int begin, int end, point p0)
{
    point min = A[begin];
    int where = begin;
    int n;
    for (n = begin + 1; n < end; n++)
        if (polarAngle(A[n], p0) < polarAngle(min, p0))
        {
            min = A[n];
            where = n;
        }
    return where;
}
void selectionSort(point A[], int N, point p0)
{
    int n, s;
    point temp;
    for (n = 0; n < N; n++)
    {
        s = whereSmallest(A, n, N, p0);
        temp = A[n];
        A[n] = A[s];
        A[s] = temp;
    }
}

// Remove the last item from the list
void popBack(list *p)
{
    int x;
    x = p->size - 1;
    p->A[x] = p->A[x + 1];
}

// Return the last item from the list
point getLast(list *p)
{
    point value;
    value = p->A[p->size];
    return value;
}

// Return the next to the last item
point getNextToLast(list *p)
{
    point value;
    value = p->A[p->size - 1];
    return value;
}

int main(int argc, const char *argv[])
{
    point p0, P;
    FILE *input;
    list *p;
    int N, n, x, y;

    /*Assuming that the first piece of data in the array indicates the amount of numbers in the array then we record this number as a reference.*/
    N = 0;
    input = fopen("points.txt", "r");
    fscanf(input, "%d", &N);

    /*Now that we have an exact size requirement for our array we can use that information to create a dynamic array.*/
    p = (point*)malloc(N*sizeof(point));
    if (p == NULL)//As a safety precaution we want to terminate the program in case the dynamic array could not be successfully created.
        return -1;

    /*Now we want to collect all of the data from our file and store it in our array.*/
    for (n = 0; n < N; n++)
    {
        fscanf(input, "%d %d", &P.x, &P.y);
        p->A[n] = P.x;
        p->A[n] = P.y;
    }
    fclose(input);

    free(p);
    return 0;
}

Answer 1

First of all, your code cannot be compiled because this

p->A[n] = P.x;
p->A[n] = P.y;

is wrong, it should be

p->A[n].x = P.x;
p->A[n].y = P.y;

because A has type point and you should access the members of the struct in order to assign values to them.

But this is just the begining of the problems, you didn't allocate space for the A pointer, so this will not work.

1. You need to allocate space for an instance of type list, which is done this way

   p = malloc(sizeof(*p));
   

2. Then you need to initialize p's members, for which

   p->values   = malloc(N * sizeof(point));
   p->capacity = N;
   p->size     = 0;
   

   as you see space was allocated for the values member.

3. Check fscanf() to insure data integrity and avoid undefined behavior, if fscanf() fails you would never know with your code and you potentially access uninitialized variables which leads to Undefined Behavior.

4. Capture the values scanned from the file in two int variables and copy them to the array only if the where sucessfuly read

   for (n = 0 ; ((n < N) && (fscanf(input, "%d%d", &x, &y) == 2)) ; n++)
   /* check that the values were read from the file _______^ */
   {
       /* store them in the array */
       p->values[n].x = x;
       p->values[n].y = y;
       p->size       += 1;
   }
   

5. Check that the file did open.

I suggest the following code

#include <stdio.h>
#include <math.h>
#include <stdlib.h>

typedef struct
{
    int x;
    int y;
} point;

typedef struct
{
    int size;
    int capacity;
    point *values;
} list;

// Compute the polar angle in radians formed
// by the line segment that runs from p0 to p
double polarAngle(point p, point p0)
{
    return atan2(p.y - p0.y, p.x - p0.x);
}

// Determine the turn direction around the corner
// formed by the points a, b, and c. Return a
// positive number for a left turn and negative
// for a right turn.
double direction(point a, point b, point c)
{
    return (b.x - a.x)*(c.y - a.y) - (c.x - a.x)*(b.y - a.y);
}

int whereSmallest(point values[], int begin, int end, point p0)
{
    point min = values[begin];
    int where = begin;
    int n;
    for (n = begin + 1; n < end; n++)
        if (polarAngle(values[n], p0) < polarAngle(min, p0))
        {
            min = values[n];
            where = n;
        }
    return where;
}
void selectionSort(point values[], int N, point p0)
{
    int n, s;
    point temp;
    for (n = 0; n < N; n++)
    {
        s         = whereSmallest(values, n, N, p0);
        temp      = values[n];
        values[n] = values[s];
        values[s] = temp;
    }
}

// Remove the last item from the list
void popBack(list *p)
{
    int x;
    x = p->size - 1;
    p->values[x] = p->values[x + 1];
}

// Return the last item from the list
point getLast(list *p)
{
    point value;
    value = p->values[p->size];
    return value;
}

// Return the next to the last item
point getNextToLast(list *p)
{
    point value;
    value = p->values[p->size - 1];
    return value;
}

int main(int argc, const char *argv[])
{
    FILE *input;
    list *p;
    int   N, n, x, y;

    /*Assuming that the first piece of data in the array indicates the amount of numbers in the array then we record this number as a reference.*/
    N     = 0;
    input = fopen("points.txt", "r");
    if (input == NULL)
        return -1;
    if (fscanf(input, "%d", &N) != 1)
    {
        fclose(input);
        return -1;
    }

    p = malloc(sizeof(*p));
    if (p == NULL)
        return -1;

    /*Now that we have an exact size requirement for our array we can use that information to create a dynamic array.*/
    p->values   = malloc(N * sizeof(point));
    p->capacity = N;
    p->size     = 0;
    if (p->values == NULL)//As a safety precaution we want to terminate the program in case the dynamic array could not be successfully created.
    {
        free(p);
        fclose(input);

        return -1;
    }

    /*Now we want to collect all of the data from our file and store it in our array.*/
    for (n = 0 ; ((n < N) && (fscanf(input, "%d%d", &x, &y) == 2)) ; n++)
    {
        p->values[n].x = x;
        p->values[n].y = y;
        p->size       += 1;
    }
    fclose(input);

    free(p->values);
    free(p);
    return 0;
}

As you can see there is another improvement you can do to the code, it's not too important but it would avoid using the N and n variables which are not necessary.

Note: before using a function, try to read throughly it's documentation, that will prevent all sorts of unexpected results, for example fscanf(), will help you understand my fixes more.

Answer 2

The variable p should be list p.

The points array allocation is p.A = (point*)malloc(N*sizeof(point));

In the filling loop, since A[n] is a point you can't assign it the int P.x or P.y. You can directly put values into the A[n] point like that:

for (n = 0; n < N; n++)
{
    fscanf(input, "%d %d", &(p.A[n].x), &(p.A[N].y));
}

The size and capacity of the list should be initialized: p.capacity = N; right after succesfull memory allocation and p.capacity = n; after filling the array

And in the end you should call free(p.A) instead of free(p).