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'
$