-1

I created two structs to store values in.

struct pair {
    int x_pos;
    int y_pos;
};

struct coordinates_header {
    int length;
    struct pair data[1000];
};

typedef struct coordinates_header coordinates;
coordinates *coords;

I then try to store data from a file using

char line[max_read];
int x, y;
FILE *in_file = fopen(filename, "r");
int i = 0;
coordinates *new = (coordinates *)coords;
while (fgets(line,max_read,in_file) != NULL) {
        sscanf(line,"%d %d", &x, &y);
        new -> data[i].x_pos = x;
        new -> data[i].y_pos = y;
        i++;
    }
new -> length = i;

Then I try to print out the values

int print_struct(void *coords) {
    coordinates *new = (coordinates *)coords;
    for (int i = 0; i < new -> length; i++) {
        printf("%d, %d\n", new->data[i].x_pos, new->data[i].y_pos);
    }
    return 0; 
}

And then I get a segmentation fault I was wondering if someone could point out where the error is. I have no experience with void but require the flexibility for the structure in some functions I'm going to use.

The file read will have the form

100 30
50 200
.. ..
Kayanda
  • 9
  • 3
  • 2
    1) `i += 2;` --> `i++;` 2) `i <= new -> length;` --> `i < new->length;` – BLUEPIXY Jan 14 '17 at 04:40
  • 3
    Show definition of `coords` and `coordinates` – BLUEPIXY Jan 14 '17 at 04:42
  • 2
    See [ask] and provide a [mcve]. And don't use things you don't understand. `void *` should only be used under specific circumstances and from the code, your's are not one of them. Don't dfy type-checking, your compiler is your friend, not your foe! – too honest for this site Jan 14 '17 at 04:54
  • Sorry forgot to add this – Kayanda Jan 14 '17 at 05:17
  • typedef struct coordinates_header coordinates; – Kayanda Jan 14 '17 at 05:17
  • 2
    You still have not shown what `coords` is. Please take the time to read how to post a [mcve]. We need to be able to copy your code *exactly* as shown and run it to see the same problem as described. – kaylum Jan 14 '17 at 05:24
  • Some of the code was provided by the instructor including void so that it is compatible with all students, hopefully this explains any weird code decisions. Also I just started to learn c a week ago, so if you have any advice on how to deal with void in the future, I'd be grateful. Thanks – Kayanda Jan 14 '17 at 05:42
  • `coordinates *coords;` declares an uninitialized pointer. You must first allocate data block with `malloc()` and then assign the pointer to it to the variable `coords`. – DYZ Jan 14 '17 at 05:43
  • would sizeof(coordinates_header) be the correct byte size? – Kayanda Jan 14 '17 at 05:47
  • Is `filename` just a pair of `x y` coords on each line? – RoadRunner Jan 14 '17 at 05:49
  • 1. You need to set `coords` to point to a valid buffer. 2. `i <= new->length` needs to be `i < new->length` 3. You need to learn to use a debugger. – kaylum Jan 14 '17 at 05:52
  • There is no point in having a pointer to `coords` (which is the reason for the segfault). Just declare a static variable `coordinates coords;` (no asterisk!) and replace all `->` with `.`. – DYZ Jan 14 '17 at 05:53
  • Just a nit (more style to avoid confusion). You will want to avoid using `new` as a variable name as it is an allocation function in C++. Yes, it is technically OK in C, but for someone scanning your code, you may create a bit of confusion. As it is a style/readability issue -- it is completely up to you. – David C. Rankin Jan 14 '17 at 08:37

1 Answers1

1

I believe their are some mistakes in your code:

  • Instead of using coordinates *coords;, which is just a dangling pointer not pointing anywhere in memory, you should just declare a structure member coordinates coords.
  • Their is no need for void* pointers in your code. You would be better off using coordinates *coords to access the address of the structure member coordinates coords, instead of void *coords.
  • You are not checking the return value of FILE *in_file, which could return NULL if not opened properly.
  • It is always good to check the result of sscanf(), just incase two x and y coordinates were not found on a line.

With these recommendations, you can write your code like this:

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

#define NUMCOORDS 1000
#define MAXREAD 100

typedef struct {
    int x_pos;
    int y_pos;
} coords_t;

typedef struct {
    coords_t coords[NUMCOORDS];
    int length;
} coordinates_t;

void print_struct(coordinates_t *coordinates);

int main(void) {
    coordinates_t coordinates;
    char line[MAXREAD];
    FILE *in_file;
    int i = 0;

    in_file = fopen("coords.txt", "r");
    if (in_file == NULL) {
        fprintf(stderr, "Error reading file.\n");
        exit(EXIT_FAILURE);
    }

    while (fgets(line, MAXREAD, in_file) != NULL) {
        if (sscanf(line, "%d %d", &coordinates.coords[i].x_pos,
                                  &coordinates.coords[i].y_pos) != 2) {

            fprintf(stderr, "two coordinates(x, y) not found.\n");
            exit(EXIT_FAILURE);
        }
        i++;
    }
    coordinates.length = i;

    print_struct(&coordinates);

    fclose(in_file);

    return 0;
}

void print_struct(coordinates_t *coordinates) {
    int i;

    for (i = 0; i < coordinates->length; i++) {
        printf("%d, %d\n", coordinates->coords[i].x_pos, coordinates->coords[i].y_pos);
    }
}
RoadRunner
  • 25,803
  • 6
  • 42
  • 75