0

I'm attempting to read from a text file that will have its first value being the amount of entries within the text. With this value, i will create a for loop that will assign the date and texts into a specific struct, until all entries have been placed in a struct. It will also print the values through each for loop. However, when compiling, it gives segmentation fault: 11. Could you please explain, i'm not very good at structs and malloc. Thank you in advance.

(Please note, the text dates printed are intentionally different to those in the text file for my assignment).

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

int main(int argc, char* argv[])
{

FILE* journal;
int i, numentries;

Entry* entries;
Entry* temp;

if (argc != 2)
{
    printf("Index required");
}

fscanf(journal, "%d", &numentries);
entries = (Entry*)malloc((numentries)*sizeof(Entry));


for(i=0; i<numentries; i++)
{
    fscanf(journal,"%2d/%2d/%4d", &entries[i].day, &entries[i].month, &entries[i].year);
    fgets(entries[i].text, 101, journal);
    printf("%4d-%2d-%2d: %s", entries[i].year, entries[i].month, entries[i].day, entries[i].text);
}

fclose(journal);
return 0;
}

with my header file (journal) being ->

typedef struct {
    int day;
    int month;
    int year;
    char text[101];
}Entry;

Entry entries;

An example of the text file will be:

2
12/04/2010
Interview went well i think, though was told to wear shoes.
18/04/2010
Doc advised me to concentrate on something... I forgot.
L.Kay
  • 61
  • 4
  • 2
    You don't (f)open `journal` – Kninnug Oct 05 '17 at 08:32
  • 1
    you should use fopen method for file to be open – Mostch Romi Oct 05 '17 at 08:33
  • 1
    You also forgot to handle the `\n` after the date, so the first `fgets` will only read the new line character. https://ideone.com/9Vi9tU – mch Oct 05 '17 at 08:35
  • 1
    Please check the return value of `fscanf()` to make sure `numentries` is valid. I/O can fail. Also the allocation ca be written `entries = malloc(numentries * sizeof *entries);` which I argue is simpler and better. [Don't cast the return value of `malloc()` in C](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – unwind Oct 05 '17 at 08:37
  • 1
    Or [**do** cast](https://stackoverflow.com/a/14879184/1312382) (from same question...) - there are both sides, one in favour (admitted, the majority), and a respectable minority. My recommendation: read the arguments of both sides and *make up your own mind* (I did so an I *do* cast...). – Aconcagua Oct 05 '17 at 08:47
  • Side note: you shouldn't declare variables in header files (`Entry entries;` belongs to the C file). – Jabberwocky Oct 05 '17 at 08:58

1 Answers1

1

This is a minimal example that works:

Modifications:

  • opening file and check if file could not be opened
  • corrected input format strings (added \n in fscanf)

Opional modifications (program works also without them):

  • removed cast from malloc
  • declared variables where they are used
  • removed useless variables
  • using sizeof(*entries) instead of sizeof(Entry) in malloc
  • using sizeof(entries->text) instead of hard coded value 101

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

typedef struct {
  int day;
  int month;
  int year;
  char text[101];
}Entry;

Entry entries;

int main(int argc, char* argv[])
{
  FILE* journal = fopen("yourfile", "r");
  if (journal == NULL)
  {
    printf("Cannot not open file\n");
    return 1;
  }

  int numentries;
  fscanf(journal, "%d", &numentries);
  Entry* entries = malloc(numentries * sizeof(*entries));

  for (int i = 0; i<numentries; i++)
  {
    fscanf(journal, "%2d/%2d/%4d\n", &entries[i].day, &entries[i].month, &entries[i].year);
    fgets(entries[i].text, sizeof(entries->text), journal);
    printf("%4d-%2d-%2d: %s", entries[i].year, entries[i].month, entries[i].day, entries[i].text);
  }

  fclose(journal);
  return 0;
}

There is still no error checking performed except for the case the file cannot be opened. This is left as an exercise for the reader.

Jabberwocky
  • 48,281
  • 17
  • 65
  • 115