strange crash on fscanf

Question

So, this is my very small code which I was about to start but FOR AN UNKNOWN reason it crashes on scanf. I checked with printf and that's the place where it crashes. I've tried a lot of things and I can avoid this problem but I just want to know what's wrong if you could please tell me.

For example this can be datafile:

19/03/2017 Good
17/03/2017 Terrible
18/03/2017 Good

Here is the code:

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

#define DATA_FILE "Database.txt"
#define MAX_USERS 50

typedef struct data {
    char date;
    char feeling;
} data;

int ReadData(data *p1, FILE *fp);

int main() {
    FILE *fp = fopen(DATA_FILE, "r");
    data *p1;
    p1 = (struct data*)malloc(MAX_USERS * sizeof(struct data));
    if (fp == NULL) {
        printf("Database is empty!");
        exit(1);
    }
    ReadData(p1, fp);
}

int ReadData(data *p1, FILE *fp) {
    int i = 0;
    while (!feof(fp)) {
        fscanf(fp, "%s %s", (p1 + i)->date, (p1 + i)->feeling);
        printf("%s %s", &(p1 + i)-> date, &(p1 + i)-> feeling);
        i++;
    }
    return i;
}

Answer 1

You have a mismatch between how you define date and feeling in your struct type:

typedef struct data
{
    char date;
    char feeling;
}data;

and how you're treating them in the scanf call:

fscanf(fp, "%s %s", (p1+i)->date, (p1+i)->feeling); 

Remember that %s expects its corresponding argument to have type char *, and to point to the first element of an array of char, not a single char. You've declared date and feeling to be single chars. And, (p1+i)->date does not evaluate to the address of the date member within the instance.

For this to work as you are intending it to, both date and feeling need to be declared as either arrays of char:

typedef struct data
{
    char date[MAX_DATE_LENGTH+1];        // where MAX_DATE_LENGTH and MAX_FEELING_LENGTH
    char feeling[MAX_FEELING_LENGTH+1];  // are constant integral expressions; +1 to account
}data;                                   // for string terminator

or, you need to define date and feeling as pointers to char and allocate memory for them later:

typedef struct data
{
    char *date;
    char *feeling;
}data;
...
p1[i]->date = malloc( MAX_DATE_LENGTH + 1 );
p1[i]->feeling = malloc( MAX_FEELING_LENGTH + 1 );

Now, your scanf call will work with one minor tweak:

fscanf(fp, "%s %s", p1[i].date, p1[i].feeling);

(p1 + i)->date works, but I think using regular subscript notation is easier to read and follow.

Answer 2

There are multiple problems in your code:

• you should define the date and feeling members of structure data as arrays of characters instead of single char types. For example:

  typedef struct data {
      char date[20];
      char feeling[40];
  } data;
  

• Using the pointer syntax (p1 + i) -> date is considered much less readable than the equivalent array syntax: p1[i].date.

• The arguments to fscanf are incorrect: you must pass the address of a character array for format %s. You pass the value of the members instead of their addresses, a very probable cause for the crash. Passing the address of single char members would be incorrect a single char cannot hold a string. With the updated structure definition, you should write:

  fscanf(fp, "%19s %39s", p1[i].date, p1[i].feeling);
  

  p1[i].date is now an array, so passing it as an argument actually passes the address of it first element.

  Specifying a width of 19 tells fscanf() to not store more than 19 characters before the null terminator into the destination array.

• while(!feof(fp)) is always the wrong way to test for end of file. Instead check that fscanf() succeeds at converting 2 strings:

  while (fscanf(fp, "%19s %19s", p1[i].date, p1[i].feeling) == 2) { ...
  

• You pass the address of single char values to printf for the %s specifier. This is incorrect. With the updated structure definition, use this:

  printf("%s %s", p1[i].date, p1[i].feeling);
  

• You should stop reading the file when you have converted MAX_USERS entries.

• You should check for memory allocation failure

• You should free the memory block

• You should close the file

• Parsing the date as a single word is probably OK, but the feeling should allow multiple words, as in Very bad and So so. Use format %[^\n] for that.

Here is a corrected version:

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

#define DATA_FILE "Database.txt"
#define MAX_USERS 50

typedef struct data {
    char date[20];
    char feeling[40];
} data;

int ReadData(data *p1, FILE *fp);

int main(void) {
    FILE *fp;
    data *p1;
    int data_count;  /* number of records read */

    fp = fopen(DATA_FILE, "r");
    if (fp == NULL) {
        printf("Database not found\n");
        exit(1);
    }

    p1 = malloc(MAX_USERS * sizeof(struct data));
    if (p1 == NULL) {
        printf("Cannot allocate memory\n");
        fclose(fp);
        exit(1);
    }
    data_count = ReadData(p1, fp);

    /* Do something with the data */

    free(p1);
    fclose(fp);
    return 0;
}

int ReadData(data *p1, FILE *fp) {
    int i;
    for (i = 0; i < MAX_USERS; i++) {
        if (fscanf(fp, "%19s %39[^\n]", p1[i].date, p1[i].feeling) != 4)
            break;
        printf("%s %s", p1[i].date, p1[i].feeling);
    }
    return i;
}

Answer 3

Your date and feeling fields in your database are strings. You are reading them as strings by specifying %s in the fscanf format. But you are trying to store them in a memory location of just 1 byte length.

Replace

typedef struct data
{
    char date;
    char feeling;
}data;

with

typedef struct data
{
    char date[11];
    char feeling[20];
}data;

Answer 4

char date; is a single char. You'll need something like char date[20]; to use the format spec %s.