2

Im currently working on a problem to store data from an input file into a struct of the following format:

typedef struct school_{
  char *name;
  char *state;
}School;

Im reading from an input file of the format :

name1, state1
name2, state2

And I would like to store the data dynamically for each school in a struct via pointers as the length of the name is unknown. k is the number of lines in the file. So far this is what I have:

void input_schools(FILE *IN, School **Sch, int k) { 
  int i, j;
  char ch;

  for (i=0; i<k; i++)
{ fscanf(IN, "%c", &ch);
  Sch[i].name = (char *)malloc(sizeof (char));

  j = 0;
  Sch[i].name[j] = ch;

  while(ch != '-') {
    fscanf(IN, "%c", &ch);
    j++;
    Sch[i].name = (char *) realloc(Sch[i].name, sizeof(char)*(j+1));
    Sch[i].name[j] = ch;
  }

}
Sch[i].name[j-1] = '\0';

However I am receiving a seg fault which I'm assuming is from the way I'm trying to store "ch" when writing "Sch[i].name[j]" I have also tried Sch[i]->name[j] and been unsuccessful. I would appreciate any help in knowing the correct way to write the address to store the data?

I call the function using : input_schools(school_info,TOP100,school_size); where school info is the input file School *TOP100[school_size]; is top100 and school_size is the number of lines in the file

codingstruggles
  • 39
  • 2
  • 1
    `School **Sch` so `Sch[i]` is a pointer - `Sch[i].name` should produce a compiler error. Show how you call the function. – 001 Nov 13 '18 at 19:40
  • 4
    Also, c-strings need to be terminated with `\0` which you neglect to do. – 001 Nov 13 '18 at 19:43
  • 1
    Try and give your variables meaningful names, *especially* as arguments. What is `k`? – tadman Nov 13 '18 at 19:43
  • 2
    Hint: `strdup` instead of manually copying strings. – tadman Nov 13 '18 at 19:44
  • yap. try better names, and use a better indentation and formatting. Probably install a linter, it checks for formatting & compilation errors as you type. Also, there are some nice naming conventions out there. for example, instead of `k`, I would write `a_number_of_lines`. The`a_` tells me that the variable is a function argument, and the remaining part tells me what the variable has. Also, unless you need, declare the variables in the loop. `for(int i = 0; i < a_numer_of_lines; i++)` – Aravind Voggu Nov 14 '18 at 04:37
  • @codingstruggles This question is still listed as unanswered. Doesn't any of the supplied answers answer it? – Ted Lyngmo May 25 '19 at 10:01

2 Answers2

1

Your file is very similar in shape to a csv. See if you can use any csv parsing libraries or code.

Instead of checking every char, read a whole line into a buffer and use strtok. strtok is a function used to split a string by a delimiter. a ',' in your case.

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

const char* getfield(char* line, int num)
{
    const char* tok;
    for (tok = strtok(line, ",");
            tok && *tok;
            tok = strtok(NULL, ",\n"))
    {
        if (!--num)
            return tok;
    }
    return NULL;
}

int main()
{
    FILE* stream = fopen("in.md", "r");

    char line[1024];
    while (fgets(line, 1024, stream))
    {
        char* tmp1 = strdup(line);
        char* tmp2 = strdup(line);

        printf("Name is %s\n", getfield(tmp1, 1));
        printf("State is %s\n", getfield(tmp2, 2));
        // NOTE strtok changes the string. Hence two vars. You can try duplicating in the function instead.
        // Note that I'm freeing the data. copying with strdup instead of directly assigning may be wise.
        free(tmp1);
        free(tmp2);
    }
}
Aravind Voggu
  • 1,491
  • 12
  • 17
  • Sadly I have to use the format of the struct above. if I used strtok to get each string is there a way to store them in the struct formatted as shown above? – codingstruggles Nov 13 '18 at 21:13
  • 1
    There are, but I advise you go with the answer by Ted Lyngmo. It's much easier to understand and implement. You can use just the fscanf part, you don't need to use the other part, but it's generally good. – Aravind Voggu Nov 14 '18 at 04:31
1

You could use something like this to read one School entry from the FILE* you supply:

bool School_read(School* s, FILE* in) {

    int scan = fscanf(in, " %m[^,\n], %m[^\n]", &s->name, &s->state);

    // the fscanf format string:
    // <space> = skip leading whitespaces (like a newline from the line before)
    // %m[^,\n] = read a string until, but not including, "," or "\n"  m = allocate space for it
    // , = expect a comma and discard it
    // %m[^\n] = read a string until, but not including, "\n" and allocate space for it

    // just a debug print
    fprintf(stderr, " -- got %d hits, >%s< >%s<\n", scan, s->name, s->state);

    if(scan<2) {
        // not a complete scan, failure
        if(scan==1) {
            // apparently, we got one match, free it
            free(s->name);
            s->name = NULL;
        }
        return false;
    }
    return true;
}

I don't know how widespread the support for the 'm' modifier that dynamically allocates memory for the strings is though. Recent gcc and clang compilers supports it anyway.

You could also make functions for creating and destroying a School:

School* School_create() {
    School* s = malloc(sizeof(School));
    if(s!=NULL) {
        s->name = NULL;
        s->state = NULL;
    }
    return s;
}

void School_destroy(School** sp) {
    if(sp) {
        School* s = *sp;
        if(s) {
            if(s->state) free(s->state);
            if(s->name) free(s->name);
            free(s);
        }
        *sp = NULL;
    }
}

..and combine them all:

School* School_create_and_read(FILE* in) {
    School* s = School_create();
    if(s) {
        if(School_read(s, in)==false) {
            School_destroy(&s);
        }
    }
    return s;
}

So in your function populating the array of schools:

void input_schools(FILE* IN, School** Sch, int k) { 
    School* s;
    while( (s=School_create_and_read(IN)) ) {
        // s is a valid School pointer
        // store it in your array 
    }               
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108