1

I want to read a txt file, line by line, and each line stores in a different variable: here is the txt file I want to read

Jenny
Woodbridge Ave 
Amber
Exeter street
Michael
Main Street
David
Plainfield ave

and I did like


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

typedef struct info
{
   char name[20];
   char add[50];
}INFO;

int main(void){
    const char *fileName = "test.txt";
    FILE *file = fopen(fileName, "r");
    INFO* list = (INFO*)malloc(20*sizeof(INFO));


    readFromFile(file,list);
    fclose(file);
    free(list);
   return 0;
}
void readFromFile(FILE *file,INFO* list){
     int i = 0;
     while(!feof(file)){
      fscanf(file,"%s %s\n ",(list+i)->name,(list+i)->adds);
      i++;
     }
}

but I getting

Name:           Jenny
Addr:   Woodbridge
------------------------------
Name:           Ave
Addr:   Amber
------------------------------
Name:           Exeter
Addr:   street
------------------------------
Name:           Michael
Addr:   Main
------------------------------
Name:           Street
Addr:   David
------------------------------
Name:           Plainfield
Addr:   ave

I just edited a little bit so I need to use fgets to read line by line instead of fscanf() right?

Boba
  • 87
  • 10
  • 1
    You could tell the computer to do the thing you want it to do. Right now you're telling it to do something different and it's doing what you told it to. – user253751 Sep 28 '20 at 15:09
  • Please post a [mcve]. Especiall we have no idea how you call `read`. Also you shouldn't use the name `read`as there is already a standard function thet ha s the same name, which may cause other problems in the future. – Jabberwocky Sep 28 '20 at 15:09
  • Oh and `%s %d %s` doesn't match the parameters. You probably want `%s %s` – Jabberwocky Sep 28 '20 at 15:11
  • https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong – William Pursell Sep 28 '20 at 15:12
  • 1
    Firstly, your usage of `while(!feof(file))` is [wrong](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) and you will have to check if readings are successful **before** using what are "read". Secondly, It seems your file have nothing to read via `%d`. Thirdly, *undefined behavior* is invoked because there are 3 input directives while only 2 pointers are given. – MikeCAT Sep 28 '20 at 15:12
  • 1
    If you want to read lines, you should not be using scanf. You ought to use `fgets`. If you do use `scanf`, you must not use `"%s"`. – William Pursell Sep 28 '20 at 15:15
  • @MikeCAT Thanks a lot, I didn't know that at all. It seems like what my professor taught was wrong. she told us using `while(!feof(file))` to check loop condition... – Boba Sep 28 '20 at 15:35

2 Answers2

0

%s specifier reads the input stream until it finds a blank character, in your case as you have 2 words per address, it becomes umbalanced as soon as you try to read it, the second word of the address is read by the next cycle into name, it also has a problem of potencial buffer overflow.

You should use %49[^\n], this specifier reads everything until it finds the newline character, including spaces. The 49 is meant to limit the size of the read line as to avoid the mentioned buffer overflow, in you case you have space for 50 characters, the last character would be for the null terminator.

feof is also not the best way to signal the end of file in this kind of routine, more info in Why is “while ( !feof (file) )” always wrong?.

There are some other issues I address in the comments on the below working sample:

Online demo

int main()
{
    //casting malloc may make you program fail, it hides the lack of #include <stdlib.h>
    INFO* list = malloc(20 * sizeof *list); //using the the dereferenced variable 
                                            //is safer regarding future code refactoring
    const char *fileName = "test.txt";
    FILE *f = fopen(fileName, "r"); //dont forget to verify the return of fopen
    int i = read(f, list);

    for (int j = 0; j < i; j++) //test print
    {
        printf("Name: %s    Address: %s\n", list[j].name, list[j].add);
    }
}
int read(FILE *file, INFO *list)
{
    int i = 0;
    //use scanf as stop condition, [] notation is easier to read
    //limiting the size in scanf specifiers avoids buffer overflow
    while (fscanf(file, "%19s %49[^\n]", list[i].name, list[i].add) == 2)
    {
        i++;
    }
    //return i so that you know how many structs were read
    return i;
}

Output:

Name: Jenny    Address: Woodbridge Ave 
Name: Amber    Address: Exeter street
Name: Michael    Address: Main Street
Name: David    Address: Plainfield ave
anastaciu
  • 23,467
  • 7
  • 28
  • 53
0

so I need to use fgets to read line by line instead of fscanf() right?

fscanf(file,"%s %s\n ",.... fails as %s does not read spaces into a string as needed by "Woodbridge Ave" and fails to protect against buffer overrun.

There are many way to solve this task. fgets() is the most clear.

Consider a helper function to read a line and handle fgets() peculiarities of line input. Adjust as needed.

// return 1 on success
// return EOF on end-of-file/input error
// else return 0
int read_line(FILE *file, int sz, char *line) {
  if (fgets(line, sz, file) == NULL) {
    return EOF; // EOF or rare input error
  }
  int len = strlen(line);
  if (len > 0 && line[len - 1] == '\n') {
    line[--len] = '\0'; // lop off potential \n
  } else if (len + 1 == sz) { // no \n read, `line` full, so look for rest of input
    int ch;
    int extra = 0;
    while ((ch = fgetc(file)) != '\n' && ch != EOF) {
      extra = 1;
    }
    if (extra) {
      return 0;  // input too long
    }
  }
  return 1;
}

Now read the file in line pairs into a INFO

int read_INFO(FILE *file, int n, INFO *list) {
  int i = 0;
  while (i < n 
      && read_line(file, sizeof list->name, list[i].name) == 1
      && read_line(file, sizeof list->add, list[i].add) == 1) {
    i++;
  }
  return i;
}

Usage

  int n = 20;
  INFO *list = malloc(sizeof *list * n);
  if (list) {
    int number_read = read_INFO(file, n, list);
    if (number_read > 0) {
      // The happy path
    }
  }
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256