Correct way to limit document read in C without !feof, document error cannot read the .txt

Question

I've been doing this C program which requires reading .txt files and so on. There's been lots of warning about using !feof but I still don't understand where the limitations !feof could bring. I wonder if the fault on my code today is really on !feof?

typedef struct City {
  char cityName[20];
  char cityID[10];
};

void readFiles() {
  //preparing .txt file to read
  char *txtMap = "map.txt";
  char *txtPrice = "deliver_price.txt";
  FILE *fmap = fopen(txtMap, "r");
  FILE *fprice = fopen(txtPrice, "r");
  City cityArr[20];                     //I've defined the typedef struct before
  int j, a = 0;

  if (fmap == NULL || fprice == NULL || fmap && fprice == NULL) {
    if (fmap == NULL) {
      printf("\n\n\n\t\t\t\t\tError: Couldn't open file %s\n\n\n\n\n\n\n",
          fmap);
      printf("\n\n\n\t\t\t\t\tPress enter to continue\n\t\t\t\t\t");

      return 1;
    } else if (fprice == NULL) {
      printf("\n\n\n\t\t\t\t\tError: Couldn't open file %s\n\n\n\n\n\n\n",
          fprice);
      printf("\n\n\n\t\t\t\t\tPress enter to continue\n\t\t\t\t\t");

      return 1;
    }

  }

  while (!feof(fmap)) {
    City newCity;

    fscanf(fmap, "%[^#]||%[^#]\n", &newCity.cityName, &newCity.CityID);
    cityArr[a] = newCity;
    a++;
  }
  printf("reading file succesfull");
  fclose(fmap);

  for (j = 0; j < a; j++) {
    printf("\n%s || %s\n", cityArr[j].cityName, cityArr[j].cityID);
  }
  getch();
}

The text files need to be read:

New York||0
Washington D.C||1
Atlanta||2
Colombus||3

This program cannot read the files properly and making the program crash returning memory number. Anyone knows what's wrong with this program?

Sometimes when I tried fixing it, it says 'this part is a pointer, maybe you meant to use ->' error stuff. I don't know why this happen because in previous code, where I copied the file processing code part from, it doesn't happen like this.

"*making the program crash*". Now would be a good time to learn to debug your own code. Run your program in a debugger. At a minimum it will give you the exact line of code that triggers the crash. Can also use it to trace the program flow and variable values leading up to that crash. — kaylum, Jun 07 '22 at 03:52
It is not productive to attempt to debug incomplete code snippets. For example, it is important to know how `City` is defined. If you need further help after attempting to debug yourself please provide a complete [mre]. — kaylum, Jun 07 '22 at 03:53
You are definitely misusing `feof()` (see [`while (!feof(file))` is always wrong](https://stackoverflow.com/q/5431941/15168) for details).. In the input loop, you need to test the return value from `fscanf()` — which then makes testing `feof()` pointless. — Jonathan Leffler, Jun 07 '22 at 03:53
answers here should help: https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong — yano, Jun 07 '22 at 03:54
`"%[^#]||%[^#]\n"` is a bad format. At best, only the `"%[^#]"` will read anything - once. — chux - Reinstate Monica, Jun 07 '22 at 03:55
Your condition `if(fmap == NULL || fprice == NULL || fmap && fprice == NULL)` doesn't need the `|| fmap && fprice == NULL` term. — Jonathan Leffler, Jun 07 '22 at 03:56
Dean Debrio, Tips: Use `fgets()` and save time by enabling all warnings. — chux - Reinstate Monica, Jun 07 '22 at 03:56
@kaylum alright I've added the struct code. For debugging part, I saw the code crash at the fscanf part. But since everything is okay from the code I copied it from, is it because the // part? — Dean Debrio, Jun 07 '22 at 07:41
@JonathanLeffler can you help me by giving some example especially the testing input one? I've read those threads which is very interesting because my class taught me with !feof but I cannot understand how to implement without it yet. — Dean Debrio, Jun 07 '22 at 07:44
@chux-ReinstateMonica I also suspect its from that format but the test required those format. Does the program mistook || as OR even in a string as well? The program at some edit yesterday did successfully take several lines but the third lines changed its char into unreadable words and some memory address. Do you know whats wrong with that format and how to use that format correctly (with /|| I presume?) — Dean Debrio, Jun 07 '22 at 07:46
@JonathanLeffler alright thanks for the redundant fmap && fprice == NULL condition, I'll erase it later. — Dean Debrio, Jun 07 '22 at 07:48
@chux-ReinstateMonica but fgets only take unformated string, is there any way to take formatted string with fgets? — Dean Debrio, Jun 07 '22 at 07:48
@DeanDebrio True that `fgets()` reads a _line_ of input and not some other format, yet you do not need to read formatted input as you have (incorrectly). Instead, since data is in _lines_, read a _line_ into a _string_ with `fgets()`, then parse with other functions such as `sscanf()`. What do you want to do with input that is not in the expected format? — chux - Reinstate Monica, Jun 07 '22 at 12:23
@chux-ReinstateMonica i see, could you show me some steps by what you mean as "parse with sscanf()?" I ought to use the data to be read in such format just for written test problem purposes, otherwise I just format it in different-but-it-work way like with #. Is it true that the || is affecting parse even if its inside string quote? What explains the formatted parser only reads some (two of the top) like what other comment says? — Dean Debrio, Jun 07 '22 at 12:50
*I've been warned so many times by the community about avoiding `!feof`* So let me see if I understand this. You've heard it said, many times, not to use `!feof()`. But you didn't believe it, and you used it anyway. Now, you *think* it might be causing you a problem. So now, you're asking us for help analyzing this problem. But why should we spend time answering? Why should we expect you to pay any more attention to us now, than when we told you before not to use `!feof()`? — Steve Summit, Jun 07 '22 at 15:20
It's really very simple. Wrong: use `feof` to detect end-of-file. Right: Check the return value from `fscanf` or `getchar` or `fgets` to detect end-of file. — Steve Summit, Jun 07 '22 at 15:22
Also, `fscanf(fmap, "%[^#]||%[^#]\n"` is just a bad idea. It's gobbledegook. You can't figure out what it does, I can't figure out what it does. It's theoretically possible for someone to figure out what it does, but it's Just Not Worth It. As others have said, it will be far, far easier to read a whole line using `fgets`, then pick it apart some other way. See [What can I use for input conversion instead of scanf?](https://stackoverflow.com/questions/58403537) for some ideas — Steve Summit, Jun 07 '22 at 15:23
@SteveSummit Aight its my bad for ignoring community warning with a thread explaining why !feof is bad because I need it to be done quick that time. I've learned my bad in this question thanks to all people here. Yes I also agree with you that formatting is so hard to read but I was taught that way and never realized it before this error that theres other, even better, way to do formatted output. I did looked up for fgets with preparing buffer and stuff but I still don't understand what it means. Thats why I look for clue in this question and I'm glad someone show me how its actually done. — Dean Debrio, Jun 07 '22 at 15:34

chux - Reinstate Monica · Accepted Answer · 2022-06-07T15:23:48.783

4

Code has various troubles including:

Code not compiled with all warnings enabled

Save time. Enable all warnings.

Wrong use of feof()

See Why is “while ( !feof (file) )” always wrong?.

No width limit

"%[^#]" risks reading to much into &newCity.cityName.

Wrong type

"%[^#]" matches a char *. &newCity.cityName is not a char *.

Incorrect format

"%[^#]||%[^#]\n" will only match text that begins with a non-'#' and then up to, but not including a '#') followed by a '|' - which is impossible.

Consuming more than 1 line

"\n" reads any number of lines or white space.

Code is not checking the return value of input functions

Unlimited lines

Code can attempt to read more than 20 lines, yet City cityArr[20]; is limited.

Some corrections:

  while (a < 20) {
    City newCity;

    int count = fscanf(fmap, "%19[^|]||%9[^\n]%*1[\n]",
        newCity.cityName, newCity.CityID);
    if (count != 2) break;

    cityArr[a] = newCity;
    a++;
  }

Better

  // Size line buffer to about 2x expected maximum
  #define LINE_SIZE (sizeof(struct City)*2 + 4)
  char buf[LINE_SIZE];

  while (a < 20 && fgets(buf, sizeof buf, fmap)) {
    City newCity;
    int n = 0; 
    sscanf(buf, "%19[^|]||%9[^\n] %n", newCity.cityName, newCity.CityID, &n);
    if (n == 0 || buf[n] != '\0') {
      fprintf(stderr, "Bad input line <%s>\n", buf);
      return -1;
    }
    cityArr[a] = newCity;
    a++;
  }

Wrong test

fmap && fprice == NULL is not what OP wants. Review operator precedence.

// if(fmap == NULL || fprice == NULL || fmap && fprice == NULL){
if (fmap == NULL || fprice == NULL) {

Useful to post exact errors

Not "it says 'this part is a pointer, maybe you meant to use ->' error stuff."

Return from void readFiles()?

Code attempts return 1;. Use int readFiles().

FILEs not closed

Add fclose( name ) when done with file.

edited Jun 07 '22 at 15:23

answered Jun 07 '22 at 13:07

chux - Reinstate Monica

143,097
13
135
256

Thanks this reply really helps me in lots of way, especially in that %[^#] which I just realized what it means, I thought it was a replacement for %s in tutorials. My bad really. I have several questions though if you're fine with that, what is 'enabling all warnings' in gcc and how to do that because after all this time I'm just using default warning from the IDE which is enough I think (until today at least). Also what is 19 and 9 in your formatted input? And is that if(n=0...) is what you mean by checking the input to limit the loop instead with !feof? – Dean Debrio Jun 07 '22 at 15:55
If I understand the if correctly, it means "if n is 0 (I wonder where the n will be used?) or the string buffer is empty, give 'bad input format' error" ? The 'return 1' was meant to 'if this checking is passed, return to the previous menu loop' which I did tried using 'continue' but can't since its not on the loop as the compiler says. So 'return 1' is also not what I think it is? – Dean Debrio Jun 07 '22 at 16:00
@DeanDebrio The _width_ of 19, 9 limits the number of characters read into `char cityName[20]; char cityID[10];`, leaving 1 for the appended _null character_. – chux - Reinstate Monica Jun 07 '22 at 16:30
@DeanDebrio Note that `EOF, eof()` is with a _stream_ like `fmap`. `if(n=0...)` test here is due to a _string_. `fgets(buf, sizeof buf, fmap)` is the same as `fgets(buf, sizeof buf, fmap) != NULL` and that is the replacement for `while (!feof(fmap)) { ... fscanf(fmap, ...`. – chux - Reinstate Monica Jun 07 '22 at 16:34
@DeanDebrio `if (n == 0 || buf[n] != '\0') {` details. `n == 0` implies the scanning did not reach the end of the format. `buf[n] != '\0` tests if the `n` value, set due to `"%n"`, indexes the end of the string `buf[]` or not. – chux - Reinstate Monica Jun 07 '22 at 16:37
@DeanDebrio Yes, your `return 1;` ends the function `readFiles()`. There is no "menu loop" code posted. – chux - Reinstate Monica Jun 07 '22 at 16:38
@DeanDebrio "'enabling all warnings' in gcc" --> Perhaps start with `-pedantic -Wall -Wextra -Wconversion`. There are more ... – chux - Reinstate Monica Jun 07 '22 at 16:41

Correct way to limit document read in C without !feof, document error cannot read the .txt

1 Answers1