I'm trying to fill an array with a file called data.txt. I don't know what's wrong with code. I get segmentation fault: 11 error.
#include <stdio.h>
#include <stdlib.h>
void input(int arr[]){
FILE* f;
int x, i=0;
f= fopen("data.txt","r");
while (arr[i] != EOF){
fscanf(f,"%d",&x);
arr[i] = x;
i++;
}
fclose(f);
}
int main(){
int arr[50];
input(&arr[50]);
printf("%d", arr[0]);
}
You are reading the number into x (which you are copying into arr[i]) and then comparing arr[i+1] to EOF. That is not how it has to be done.
Try this
while (fscanf(f, "%d", &arr[i]) == 1)
i++;
But this would violate so many safety constraints. Better also bound check and break early if i is greater than some limit, but that limit should be passed to the function.
Another error is with how you are passing arguments to input. Pass input(arr) instead of input(&arr[50]). If you want to use & use input(&arr[0]).
f as the first argument to fscanf()…) Why not while (fscanf(f, "%d", &arr[i]) == 1) i++;, for example? Infinite loops are bad in general. Sometimes necessary, but this isn't one of those times. Yes, you terminate if fscanf() returns 0 at some time, but you keep spinning indefinitely if it returns EOF.while (fscanf(f, "%d", &arr[i]) == 1) i++; as the loop. I'm skipping array boundary checking because the size isn't passed to the function. There are advantages to this. One is that i is not incremented when the conversion fails, so you don't have to worry about whether or not to decrement i after the loop to report how many entries were created. It also doesn't need a separate status variable.int n_cvt; while ((n_cvt = fscanf(f, "%d", &arr[i])) == 1) i++;). Note that if there were 7 values being scanned, all I'd change is the format string, the list of variables, and the value 1 to 7. You'd have to consider what to do about return values EOF, 0, 1, 2, 3, 4, 5, 6. That's hard work!This would be more nearly my version of your code:
#include <stdio.h>
#include <stdlib.h>
static int input(int size, int arr[])
{
const char file[] = "data.txt";
FILE *f = fopen(file, "r");
if (f == NULL)
{
fprintf(stderr, "Failed to open file '%s' for reading\n", file);
exit(EXIT_FAILURE);
}
int i;
for (i = 0; i < size && fscanf(f, "%d", &arr[i]) == 1; i++)
;
fclose(f);
return i;
}
int main(void)
{
int arr[50];
int num = input(50, arr);
for (int i = 0; i < num; i++)
printf("%d: %d\n", i, arr[i]);
return 0;
}
The use of static before the function is necessary to quell -Wmissing-prototypes. The main() function tells the input() function how many elements are in the array so the input() function can avoid overflowing the buffer (a stack overflow, no less). The input() function tells the main() function how many values it read, so the main() function doesn't go accessing data that was not initialized. The crucial function calls are error checked — fopen() and fscanf().
The code compiles cleanly on a Mac running macOS Sierra 10.12.4 using GCC 6.3.0 and the command line below (the source file was rf19.c):
$ gcc -O3 -g -std=c11 -Wall -Wextra -Werror -Wmissing-prototypes \
> -Wstrict-prototypes -Wold-style-definition rf19.c -o rf19
$
I generated a data file with 23 random integers between 10 and 99 in it, and the output was:
$ ./rf19
0: 48
1: 33
2: 77
3: 42
4: 78
5: 51
6: 85
7: 56
8: 55
9: 56
10: 16
11: 38
12: 39
13: 52
14: 34
15: 63
16: 20
17: 23
18: 23
19: 19
20: 39
21: 44
22: 71
$
That's not dreadfully informative, but better than nothing.
The code has deficiencies still, which I don't plan to fix — some more serious than others. For example, the file name is fixed — that's a no-no. The code in the input() function exits on error; that isn't necessarily OK. It produces an error message on standard error — that's better than standard output, but wouldn't be a good idea in a GUI application. The output wastes a lot of horizontal space; with the data shown, you could get 10 values per output line (that would use about 70 characters per line), but the printing for that is more intricate, so I didn't show it. The code treats EOF and a word or punctuation character in the data the same; that might or might not matter, depending on your application. The input simply stops after the 50th entry; maybe you need to know whether there were more entries available to read. I'd probably process the command line arguments as file names, or process standard input if no files were specified — the Unix 'filter command' idiom. I'd probably do something more exciting than just print the first fifty values. I'd probably put the file reading code in a separate function from the file open/close code.
arr[i]will never equalEOF, so you loop forever and eventually segfault. You also pass the address of the first element past the end of your array toinput(), so you're trying to write into memory you don't own from the outset.input(&arr[50])should beinput(arr)fopen()or any other open-like function). You don't check the return value fromfscanf(); that too is a mistake. (See How do we check the return values fromscanf()?)while (arr[i] != EOF)on each iteration through the loop is looking at an entry inarr[]that is not yet initialized! Suggest replacing thewhile()and the next statement with:while( 1 == fscanf(f,"%d",&x) )also the loop has not limit on the number of integers that can be read, but the actual array is declared with only 50 entries, so the while() statement should be:while( i<50 && 1 == fscanf(f,"%d",&x) )input(arr[50]);is passing the value of the (one past the end of the array) to the functioninput(). The line should be:input(arr);