2

I have a text file following this format:

Thing 1: 0 0 128
Other thing: 255 64 255
Something else: 32 32 8

I intend to add more to this file eventually but the format will remain the same. What I want to do is read everything before the colon into a string and everything after it as an integer. I've tried this:

fscanf((file = fopen("colors.txt", "r")) == NULL){
    return -1;
}

fscanf("%s: %d %d %d", colorStr, &r, &g, &b);

while(!feof(file)){
    printf("%s: %d %d %d", colorStr, r, g, b);
    fscanf(file, "%s: %d %d %d", colorStr, &r, &g, &b);
}

fclose(file);

However, I get this output:

Thing 1:: 0 0 0
0: 0 0 0
0: 0 0 0
128: 0 0 0

And so on. Ideally, the output should read like this:

Thing 1: 0 0 128
Other thing: 255 64 255
Something else: 32 32 8

How can I fix this? The colorStr, r, g, and b variables were set up earlier in the program.

user3308219
  • 157
  • 2
  • 11
  • 1
    First of all: [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). Second: don't use `fscanf`. Use `fgets` and parse the result. Third - if using `fscanf`, check it's return value. Fourth - what are your types? – Eugene Sh. Feb 07 '18 at 16:12
  • @EugeneSh. This is actually one of the few questions where `feof()` is used properly. There should be some kind of celebration. – Andrew Henle Feb 07 '18 at 16:42
  • @AndrewHenle The `fscanf` in the end is reading more than a single character. So... not really. – Eugene Sh. Feb 07 '18 at 16:47
  • I don't understand why `fscanf()` should be avoided in this context. The search result I read seems to indicate that it's intended for formatted text, which the text file follows pretty rigidly. I was generous and defined a maximum string length of 64 characters even though none of these strings are going to reach, much less exceed that length. – user3308219 Feb 07 '18 at 16:47
  • 1
    @user3308219 I'd say all of the `scanf()` functions should always be avoided as they do not handle unexpected data, often in unexpected ways. Yes, they're intended for formatted text, but formats aren't always specified well enough to be what I'll call "`scanf()`-complete", and the `scanf()` failure modes can be perverse. – Andrew Henle Feb 07 '18 at 17:00
  • @EugeneSh. I'd say that's an inherent problem with `fscanf()` itself. – Andrew Henle Feb 07 '18 at 17:04
  • @user3308219 Tip in regards to "I don't understand why fscanf() should be avoided in this context", Although paradoxical, don't use `fscanf()` until you understand why `fscanf()` should be avoided in this context. – chux - Reinstate Monica Feb 07 '18 at 19:09

1 Answers1

0

The problem with your code is that the text contains spaces, which %s does not allow.

Changing the format string to %[^:] will fix this problem.

However, the code would remain vulnerable to buffer overrun. Make sure that your format string includes the max size of colorStr to prevent it:

char colorStr[100];
fscanf(file, " %99[^:]: %d %d %d", colorStr, &r, &g, &b);

Your code uses feof(file), which is incorrect. You should put fscanf into loop header. This would let you remove the duplicate fscanf call before the loop:

while(fscanf(file, " %99[^:]: %d %d %d", colorStr, &r, &g, &b) == 4) {
    printf("%s: %d %d %d\n", colorStr, r, g, b);
}

Note the space in front of the leading % format specifier. It instructs fscanf to skip the trailing spaces and/or '\n' from the previous line.

Demo.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523