I don't understand why i get segmentation fault with a function that should only swap 2 values in a struct

Question

the read_file functin works smoothly. but whenever i call the reorder function i get the segmentation fault.

in this code i define a struct containing 2 coordinates( x and y ) and save a file containing a bunch of coordinates in the "fleet" array, using the function read_file.

the input is a txt file of this type x3 y4 x8 y4 x2 y7 x5 y2 x3 y4 x5 y2 x1 y8 x5 y2 x7 y4 x6 y2 x3 y5 x5 y4 x3 y1 x3 y2 x5 y2 x5 y7

but the "fleet" array i am passing to the reorder function only takes the first 3 couples. in this case. (3 4, 2 7, 3 4)

the function reorder should take the array and evalute just the x coordinate of 2 elements of the array and swap them if the second is bigger than the first. for example

if struct[0].x < struct[1].x then swap (struct[0].x, struct[1].x)

and then again with the following couple

if struct[1].x < struct2].x then swap (struct[1].x, struct[2].x)

i get segmentation fault even if the reorder function is made up of just these lines

void reorder(struct point *fleet)
{

    struct point *temp;
    struct point temp1;
    double c;
    int i;
    if (!(temp = malloc(sizeof(struct point)))){
        free(temp);
        puts("non va");
    }
    c  =  fleet[0].x;
    temp->x = c;

    printf("%lf\n", temp->x);
   
}

how can i fix this segmentation fault?

here's the code

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

struct point {
    double x;
    double y;
};

void Read_file(FILE *f, struct point *fleet, struct  point *hits)
{
    int i, j;
    int dim = 3;
    int dim1 = 5;
    char buf[200];



    if (!(fleet = malloc(dim * sizeof(struct point)))){
        free(fleet);
        puts("doesn't work");
    }
    if (!(hits = malloc(dim1 * sizeof(struct point)))){
        free(fleet);
        puts("doesn't work");
    }
    for (i = 0; i < dim; i++) { 
        fgets(buf, sizeof(buf), f); 
            if(sscanf(buf, "x%lf y%lf", &fleet[i].x, &fleet[i].y) != 2) {puts("non va");}   // control printf 
        printf("%lf %lf\n", fleet[i].x, fleet[i].y);

        
    }
    for (i = 0; i < dim1; i++) {fgets(buf, sizeof(buf), f); 
            if(sscanf(buf, "x%lf y%lf", &hits[i].x, &hits[i].y) != 2) {puts("non va");} // control printf 
        printf("%lf %lf\n", hits[i].x, hits[i].y);
    }
}


void reorder(struct point *fleet)
{

    struct point *temp;
    int i, j;
    if (!(temp = malloc(sizeof(struct point)))){
        free(temp);
        puts("non va");
    }
    for (i = 0; i < 3; i++ ) {
    if ( &fleet[i].x < &fleet[i + 1].x) {
        temp->x = fleet[i].x;
        &fleet[i].x = &fleet[i+1].x;
        &fleet[i+1].x = temp->x;
        }
    }
}



int main(int argc,  char *argv[])
{
    struct point *hits;
    struct point *fleet;
    struct point temp;
    int dim1 =  6;
    FILE *f;
    
    if (argc < 2) {
        puts("e niente, non sono abbaastanza gli aargomenti, mannaggia");
        }
    
    f = fopen(argv[1], "r");

    Leggi_File(f, fleet, hits);
    reorder(fleet);

    
    return 0;
}

Answer 1

The pointer fleet is uninitialized and has indeterminate value

struct point *hits;
struct point *fleet;
//...

So calling this function

reorder(fleet);

results in undefined behavior.

As for the function

void Read_file(FILE *f, struct point *fleet, struct  point *hits);

then it deals with a copy of the pointer fleet

Leggi_File(f, fleet, hits);

So changing a copy in the function does not influence on the original pointer.

You have to pass the pointer by reference that is through a pointer to the pointer.

For example

void Read_file(FILE *f, struct point **fleet, struct  point **hits);

and call it like

Leggi_File(f, &fleet, &hits);

So within the function you should write for example

if (!( *fleet = malloc(dim * sizeof(struct point)))){
    puts("doesn't work");
    return;
}

To access an element of the allocated array you can use an expression like this

( *fleet )[i].x

Also it is a good idea to initialize initially the pointers to NULL

struct point *hits = NULL;
struct point *fleet = NULL;

Also for example pay attention that this loop

for (i = 0; i < 3; i++ ) {
if ( &fleet[i].x < &fleet[i + 1].x) {
    temp->x = fleet[i].x;
    &fleet[i].x = &fleet[i+1].x;
    &fleet[i+1].x = temp->x;
    }
}

for starters does not make sense due to the comparison

&fleet[i].x < &fleet[i + 1].x

It seems you mean

fleet[i].x < fleet[i + 1].x

that is you want to compare data members x of the two elements of the array.

And the loop invokes undefined behavior if the allocated dynamically array pointed to by the pointer fleet has exactly 3 elements. Because in this case using the expression

fleet[i + 1].x
      ^^^^^

can result to accessing memory beyond the allocated array.

Answer 2

C uses pass-by-value for function parameter passing, even for pointers themselves. In your call

 Leggi_File(f, fleet, hits);

you are passing fleet and trying to assign pointer to allocated memory to that pointer, and expecting that it'll reflect that in the called function. That's not true, inside the Leggi_File, you can modify the content pointed to by the pointer fleet, but any changes made to fleet itself will not be reflected back to the caller.

So, in the next call, fleetis being passed uninitialised, causing undefined behaviour.

Solution: You need to pass a pointer to fleet to Leggi_File, and handle that accordingly.

Something like

Leggi_File(f, &fleet, hits);
             ^^--------------------- pass address to pointer

and

void Read_file(FILE *f, struct point **fleet, struct  point *hits)
{
int i, j;
int dim = 3;
int dim1 = 5;
char buf[200];



if (!(*fleet = malloc(dim * sizeof(struct point)))){
    free(*fleet);
    puts("doesn't work");
}
.
.
.