1

First off, let me tell you I'm not experienced at all with C, so bear with me please. I'm helping a friend with one of his school C exercises, but we're stuck in one part, so I'm hoping some of you more advanced programmers can help us out. The exercise is basically making a program that can read a configuration file containing a map with the location of a series of mines, and it should provide the user with an interface to make changes to the map, like adding new mines or blow up an existing mine. Each position on the map is represented by 2 coordinates x and y. These coordinates are integers and they can assume values in the range [0, 24]. Each of the map's coordinates may be empty (without a mine) or have a mine, and each mine can only assume 2 states, either armed or off. The program should start by presenting the following menu to the user:

+-----------------------------------------------------
read <filename>    - read input file
show               - show the mine map
trigger <x> <y>    - trigger mine at <x> <y>
plant <x> <y>      - place armed mine at <x> <y>
export <filename>  - save file with current map
quit               - exit program
sos                - show menu
+-----------------------------------------------------

The program then waits for user to input one of the commands from above. Now, the command we're having trouble with is the "read " one. When the user enters the read command, the program should read the file with the name "filename" and build the corresponding map. The input file will be a text-encoded file that should contain a set of pairs of coordinates that represent the locations of the mines in armed state. Mines in an off state or empty spaces are not represented in the file. Each coordinate is represented by a pair of integer values (X Y) separated by a blank space, X being the row number and Y the column number. There is no restriction regarding line breaks within the file or the order of each pair of coordinates within the file.

Example of input file:

0 1
0 2
0 3
5 9 6 4 6 5
0 4
0 5

1 4 2 3 2 6 2 9
3 4
3 9 4 9
5 2
5 3

5 4
6 9 7 6
7 2
8 3
8 6

The file must be read until the end of the file is detected. For each X coordinate represented in the file, there must be a Y coordinate. If this is not the case, it means that the file is poorly formatted. In this case, the program should show stderr stream this message: "File is corrupted".

With all that said, let me show you how far we've gotten with the piece of code we have for the read command:

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

#define SIZE 128

int main(void)
{
    char line[SIZE], file_name[SIZE], command[SIZE];

    int x[SIZE], y[SIZE], i = 0;
    while (1)
    {
        fgets(line, SIZE, stdin);

        if (!strncmp(line, "read", 4))
        {
            sscanf(line, "%s %s", command, file_name);

            FILE *fp = fopen(file_name, "r");

            while (fscanf(fp, "%d %d", &x[i], &y[i]) == 2)
            {
                printf("%d %d ", x[i], y[i]);
            }

            if (fp == NULL)
            {
                fputs("Couldn't open file.\n", stderr);
            }
        }
    }
}

We're not even sure if that's a good solution, but it is indeed working when the file is correctly formatted, as it prints out every pair of coordinates. What we don't know how to do is when the file is corrupted. Let's imagine a corrupted file like this:

0 1
0 2
0 3
5 9 6 4 6 5
0 4
0 5

1 4 2 3 2 6 2
3 4
3 9 4 9
5
5

5 4
6 9 7 6
7 2
8 3
8 6

How are we supposed to get the "File is corrupted" message with an input file like the above? We have 3 single numbers, how can we catch them?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
4d5050
  • 43
  • 5
  • Hint: Why did you use `== 2` in `fscanf(fp, "%d %d", &x[i], &y[i]) == 2`? – mkrieger1 Dec 26 '20 at 19:41
  • this loop is the key: while (fscanf(fp, "%d %d", &x[i], &y[i]) == 2) Reading a file pointer, before checking for NULL and or EoF is a bad idea! What happens if you don't read 2 numbers, and instead read only 1? Handle that case, and you're done. Of course, you won't know if there are 3 single numbers, or only one, but you'll definitely know that there is a corruption. – tpb261 Dec 26 '20 at 19:43
  • `if (fp == NULL)` should be before `while (fscanf(fp,...` – chux - Reinstate Monica Dec 26 '20 at 20:04
  • Your loop `while (fscanf(fp, "%d %d", &x[i], &y[i]) == 2) { printf("%d %d ", x[i], y[i]); }` never changes `i`, so it continually reads over the same two entries in the array. Use a `for` loop: `for (i = 0; i < SIZE && fscanf(fp, "%d %d", &x[i], &y[i]) == 2; i++) { printf("%d: (%d, %d)\n", i, x[i], y[i]); }`. – Jonathan Leffler Dec 26 '20 at 20:27
  • @tpb261 How would I do that though? We know the loop is the key, but we are unable to get a working solution. – 4d5050 Dec 26 '20 at 23:07

1 Answers1

2

The code needs more error checking. As I noted in (some now deleted) comments, you must check that the file was opened before using the file pointer; you must close the file pointer if it was opened; you should check that read operations from fgets() work; you should check that read is spelt thus and not with extra garbage characters after it; you should increment i in the loop.

Putting those changes together (along with some other code hygiene changes) leads to:

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

#define SIZE 128
enum { X_MAX = 24, Y_MAX = 24, X_MIN = 0, Y_MIN = 0 };  /* You may want to support non-square mine fields */

int main(void)
{
    char line[SIZE];
    char file_name[SIZE];
    char command[SIZE];
    int x[SIZE];
    int y[SIZE];
    int i = 0;

    while (fgets(line, SIZE, stdin) != NULL)
    {
        if (strncmp(line, "read", 4) == 0)
        {
            if (sscanf(line, "%s %s", command, file_name) != 2 ||
                strcmp(command, "read") != 0)
            {
                fprintf(stderr, "Unrecognized command '%s'\n", command);
                continue;
            }

            FILE *fp = fopen(file_name, "r");
            if (fp == NULL)
            {
                fprintf(stderr, "Failed to open file '%s' for reading\n", file_name);
                continue;
            }

            for (i = 0; i < SIZE && fscanf(fp, "%d %d", &x[i], &y[i]) == 2; i++)
            {
                if (x[i] < X_MIN || x[i] > X_MAX || y[i] < Y_MIN || y[i] > Y_MAX)
                {
                    fprintf(stderr, "(%d,%d) is outside the rectangle (%d,%d)..(%d,%d)\n",
                            x[i], y[i], X_MIN, Y_MIN, X_MAX, Y_MAX);
                    i = 0;  /* 'Delete' all points in array */
                    break;
                }
                printf("%d: (%d %d)\n", i, x[i], y[i]);
            }

            fclose(fp);
        }
        /* here, i indicates how many mines were positioned in the file */
    }
}

Warning: No compiler has been consulted about the syntactic validity of this code.

Note that error messages are reported to stderr, not stdout. Note that messages other than prompts end with a newline. It isn't 100% clear whether you want to use indexes 0..23 or 1..24 (or maybe even 0..24) — I've assumed 0..24 inclusive with the enumeration values and tests, but you can adjust the enumeration constants and validation tests as works best for the rest of your code.

Although the strncmp() test suggests that the command is probably read, the user could have typed readdress or readonly or ready and all would pass the strncmp() test. Once the command has been read by sscanf(), there is a check that the command really is 'read' and not something else.

Any format error in the input is ignored in the sense that it is not reported as a specific error; it simply stops the input loop and is silently treated as EOF. You could capture the value returned by fscanf() and report different errors for return values of 0 (invalid first number) or 1 (invalid or missing second number) or EOF (no more input — not an error). Note that fscanf() doesn't care about newlines; it will happily read one number from the end of one line and a second number from the next (non-blank) line. If you want to ensure that lines contain pairs of numbers, use fgets() and the technology described in How to use sscanf() in loops?

This code does not need to specify sizes on %s in the sscanf() because both arrays (file_name and command) are as big as line, so there cannot be overflow. In general, it is a good idea to limit the length of strings scanned to avoid overflow. You might use %127s to prevent overflows in arrays of size 128. That 'off-by-one' is unfortunate but it is also a historical feature that won't be changed. You can't specify the size to sscanf() et al in an extra argument to the function like you can with printf() et al. That's also a historical feature that won't be changed.

When you're developing your program, most of this code will be pushed into one or more functions to separate this non-negligible chunk of code from the rest of the processing. For a question on SO, this was a suitable MCVE (Minimal, Complete, Verifiable Example — or MRE or whatever name SO now uses) or an SSCCE (Short, Self-Contained, Correct Example), and putting it all in main() was not unreasonable.


We're still stuck on the same issue though. How can we check the all file first, to see if there are any single numbers, to send the "File is corrupted" message? So far, we can only loop through it, while printing out every pair, and THEN catch a single one.

You may have to read the file twice. However, if each line must contain pairs of values, then you can use fgets() to read lines and sscanf() in a loop to read pairs of numbers. You don't have to stop the loop over lines when you find an odd number of values on a line; you can report an error and continue reading more lines and processing them too. You will need to the number of errors as you processed the lines; you'll probably want to track the line numbers too for better error reporting. If you need to process lines, you can't use fscanf() — it does not care about line endings.

Incidentally, it doesn't matter whether you use "%d%d" or "%d %d" with fscanf(). The %d conversion specification skips optional white space (including newlines); the blank in the format also skips optional white space.

You could use code like this:

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

#define SIZE 128
enum { X_MAX = 24, Y_MAX = 24, X_MIN = 0, Y_MIN = 0 };  /* You may want to support non-square mine fields */

int main(void)
{
    char line[SIZE];
    char file_name[SIZE];
    char command[SIZE];
    int x[SIZE];
    int y[SIZE];
    int i = 0;

    while (fgets(line, SIZE, stdin) != NULL)
    {
        if (strncmp(line, "read", 4) == 0)
        {
            if (sscanf(line, "%s %s", command, file_name) != 2 ||
                strcmp(command, "read") != 0)
            {
                fprintf(stderr, "Unrecognized command '%s'\n", command);
                continue;
            }

            FILE *fp = fopen(file_name, "r");
            if (fp == NULL)
            {
                fprintf(stderr, "Failed to open file '%s' for reading\n", file_name);
                continue;
            }

            i = 0;
            int lineno = 0;
            int errcnt = 0;
            while (fgets(line, sizeof(line), fp) != NULL)
            {
                int offset = 0;
                int rc;
                char *buffer = line + offset;

                line[strcspn(line, "\n")] = '\0';   // Zap newline if present

                lineno++;
                while (i < SIZE &&
                       (rc = sscanf(buffer, "%d %d%n", &x[i], &y[i], &offset)) == 2)
                {
                    if (x[i] < X_MIN || x[i] > X_MAX || y[i] < Y_MIN || y[i] > Y_MAX)
                    {
                        fprintf(stderr, "(%d,%d) is outside the rectangle"
                                " (%d,%d)..(%d,%d) on line %d of file '%s'\n",
                                x[i], y[i], X_MIN, Y_MIN, X_MAX, Y_MAX, lineno, file_name);
                        errcnt++;
                        break;
                    }
                    printf("%d: (%d %d)\n", i, x[i], y[i]);
                    i++;
                    buffer += offset;
                }
                if (rc == 0)
                {
                    fprintf(stderr, "Invalid data near '%s' on line %d of file '%s'\n",
                            buffer, lineno, file_name);
                    errcnt++;
                }
                else if (rc == 1)
                {
                    fprintf(stderr, "Line %d of file '%s' contains an odd"
                            " number of coordinates ('%s')\n",
                            lineno, file_name, line);
                    errcnt++;
                }
            }

            if (errcnt != 0)
            {
                fprintf(stderr, "At least %d errors were found in file '%s'\n",
                        errcnt, file_name);
                i = 0;
            }

            fclose(fp);
        }
        /* here, i indicates how many mines were positioned in the file */
    }

    return 0;
}

Given your erroneous data file and the program mines43 created from source code mines43.c, I can get this output (using Bash as my shell):

$ mines43 <<< 'read data'
0: (0 1)
1: (0 2)
2: (0 3)
3: (5 9)
4: (6 4)
5: (6 5)
6: (0 4)
7: (0 5)
8: (1 4)
9: (2 3)
10: (2 6)
Line 8 of file 'data' contains an odd number of coordinates ('1 4 2 3 2 6 2')
11: (3 4)
12: (3 9)
13: (4 9)
Line 11 of file 'data' contains an odd number of coordinates ('5')
Line 12 of file 'data' contains an odd number of coordinates ('5')
14: (5 4)
15: (6 9)
16: (7 6)
17: (7 2)
18: (8 3)
19: (8 6)
At least 3 errors were found in file 'data'
$ mines43 <<< 'readonly data'
Unrecognized command 'readonly'
$
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Thank you for all your suggestions, I've modified our original code already. We're still stuck on the same issue though. How can we check the all file first, to see if there are any single numbers, to send the "File is corrupted" message? So far, we can only loop through it, while printing out every pair, and THEN catch a single one. – 4d5050 Dec 26 '20 at 21:29
  • You may have to read the file twice. However, if each line must contain pairs of values, then you can use `fgets()` to read lines and `sscanf()` in a loop to read pairs of numbers. You don't have to stop the loop over lines when you find an odd number of values on a line; you can report an error and continue reading more lines and processing them too. You will need a flag to indicate whether or not there were errors as you processed the lines; you'll probably want to track the line numbers too. If you need to process lines, you can't use `fscanf()` — it does not care about line endings. – Jonathan Leffler Dec 26 '20 at 21:34
  • Incidentally, it doesn't matter whether you use `"%d%d" or `"%d %d"` in `fscanf()`. The `%d` conversion specification skips optional white space (including newlines); the blank in the format also skips optional white space. – Jonathan Leffler Dec 26 '20 at 21:39