0

I have a file .txt with values of some variable. I need to read them to declarate my variables in main. What is wrong?

#include <stdio.h>
#define INPUT "input.txt"

int main (void){

    FILE *open_file = fopen(INPUT, "r");

    if (open_file == NULL){
        puts("ERROR.");
    } else {
        puts("SUCCESS.");
    }

    char *buffer;
    int size = ftell(open_file); 
    int read_file = fread(buffer,size,*open_file);
    int Integer1, Integer2;
    while (size != EOF){
        sscanf("%d%d",Integer1, Integer2);
    }


    int close_file = fclose(open_file);
    if (close_file == -1){
        puts("Error in closing file.");
    } else {
        puts("Closing file: SUCCESS.");
    }


    return 0;
}

Whats is wrong? I have to read every line of my file. For example if my file contains:

1
2

My scanf should set:

Integer1 = 1; 
Integer2 = 2;
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
joy
  • 17
  • 4
  • 1
    1st problem `fread(buffer,size,*open_file);`, `buffer` is not allocated – IrAM Dec 27 '20 at 03:01
  • Can you show me how to do it? – joy Dec 27 '20 at 03:04
  • Error messages should be informative and written to stderr. Instead of `puts("ERROR")`, try `if (open_file == NULL){ perror(INPUT);}` – William Pursell Dec 27 '20 at 03:05
  • `while (size!= EOF)` is an infinite loop, since size can never change inside the loop. – William Pursell Dec 27 '20 at 03:06
  • `ftell` on a newly opened file is probably going to return 0. `fread` expects 4 arguments, but you only pass it 3. If you fix the arguments, I expect you're going to have it read `size` values, but `size` is either zero or -1, so..... what are you trying to do? Why are you using `fread` instead of `scanf`? – William Pursell Dec 27 '20 at 03:11
  • 1
    Your edit wrecked the question — removing the call to `fread()` that you were asking about. That is not allowed — especially not once you have any answers. I've rolled back to the prior version. You might add information to the question after you get one or more answers, but even that is not necessarily a good idea. – Jonathan Leffler Dec 27 '20 at 04:14
  • Could do with an exit and reporting an error when unable to open file. – Ed Heal Dec 27 '20 at 06:41

3 Answers3

2

One problem is that after the line

char *buffer;

the variable buffer does not point to any valid memory location. It is a wild pointer.

You can either create a fixed size array like this:

char buffer[100];

or you can create a dynamically sized memory buffer, like this:

if ( fseek( open_file, 0, SEEK_END ) != 0 )
    DoSomethingToHandleError();
long size = ftell(open_file);
if ( fseek( open_file, 0, SEEK_SET ) != 0 )
    DoSomethingToHandleError();
char *buffer = malloc( size );
if ( buffer == NULL )
    DoSomethingToHandleError();

Depending on whether you used a fixed-size array or a dynamically allocated buffer, you should change the call to fread to one of the following:

fread( buffer, sizeof(buffer), open_file ); //for fixed size array

fread( buffer, size, open_file ); //for dynamically allocated buffer

Instead of using the function fread, you would probably be better off using the function fgets, especially because sscanf requires a null-terminated string as input, not binary data. The function fread will give you binary data that is not null-terminated, whereas fgets will give you a null-terminated string.

Also, change the line

sscanf("%d%d",Integer1, Integer2);

to

sscanf( buffer, "%d%d", &Integer1, &Integer2);

Before using Integer1 and Integer2 afterwards, you should also check the return value of sscanf to make sure that both integers were found in the string.

However, if you don't want to handle the memory management and reading in of the file yourself, you can simply use fscanf, like this:

if ( fscanf( open_file, "%d%d", &Integer1, &Integer2 ) != 2 )
    DoSomethingToHandleError();

But this has the disadvantage that fscanf will just give you the first two numbers that it finds in the file, and won't perform much input validation. For example, fscanf won't enforce that the two numbers are on separate lines. See the following link for more information on the disadvantages of using fscanf:

A beginners' guide away from scanf()

If you use the function fgets as I suggested, then you will need two calls to fgets to read both lines of the file. This means that sscanf will be unable to find both integers in the same string. Therefore, if you use fgets, then you will have to change your program logic a bit, for example like this:

#define MAX_INTEGERS 2
char buffer[100];
int integers[MAX_INTEGERS];
int num_found = 0;

while ( fgets( buffer, sizeof(buffer), open_file ) != NULL )
{
    int i;

    if ( strchr( buffer, '\n' ) == NULL && !feof( openfile ) )
    {
        printf( "Error: Line size too long! Aborting.\n" );
        break;
    }

    if ( sscanf( buffer, "%d", &i ) == 1 )
    {
        if ( num_found == MAX_INTEGERS )
        {
            printf(
                "Error: Found too many integers in file! This "
                "program only has room for storing %d integers. "
                "Aborting.\n",
                MAX_INTEGERS
            );
            break;
        }

        //add found integer to array and increment num_found
        integers[num_found++] = i;
    }
    else
    {
        printf(
            "Warning: Line without number encountered. This could "
            "simply be a harmless empty line at the end of the "
            "file, but could also indicate an error.\n"
        );
    }
}

printf( "Found the following integers:\n" );

for ( int i = 0; i < num_found; i++ )
{
    printf( "%d\n", integers[i] );
}

Instead of using sscanf, you may want to use strtol, as that function allows you to perform stricter input validation.

If you don't want to use the function fgets, which reads input line by line, but really want to read the whole file at once using fread, you can do that, but you would have to add the terminating null character manually.


EDIT: Since you stated in the comments that you didn't want a loop and wanted to store the individual lines each into their own named variable, then you could use the following code instead:

//This function will assume that one number is stored per line, and
//write it to the memory location that "output" points to. Note that
//the return value does not specify the number found, but rather
//whether an error occurred or not. A return value of 0 means no error,
//nonzero means an error occurred.

int get_number( FILE *fp, int *output )
{
    char buffer[100];
    int i;

    //read the next line into buffer and check for error
    if ( fgets( buffer, sizeof(buffer), fp ) == NULL )
        return -1;

    //make sure line was not too long to fit into buffer
    if ( strchr( buffer, '\n' ) == NULL && !feof( fp ) )
        return -1;

    //parse line and make sure that a number was found
    if ( sscanf( buffer, "%d", &i ) != 1 )
        return -1;

    *output = i
    return 0;
}

Now, in your function main, you can simply use the following code:

int error_occurred = 0;

if ( get_number( &Integer1 ) != 0 )
{
    printf( "Error occured when reading the first integer.\n" );
    error_occurred = 1;
}

if ( get_number( &Integer2 ) != 0 )
{
    printf( "Error occured when reading the second integer.\n" );
    error_occurred = 1;
}

if ( !error_occurred )
{
    printf( "The value of Integer1 is: %d\n", Integer1 );
    printf( "The value of Integer2 is: %d\n", Integer2 );
}
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
  • And how can I do to read each line of file? – joy Dec 27 '20 at 03:46
  • @joy: I have now added a code example to my answer. – Andreas Wenzel Dec 27 '20 at 04:20
  • Yes but every line need to be stored in a named variable. If I do a loop I can't store them – joy Dec 27 '20 at 04:52
  • @joy: I have now added another code example, which does not use a loop and allows you to keep the named variables. – Andreas Wenzel Dec 27 '20 at 05:16
  • Corner case: As the last "line", may or may not contain a `'\n'`, `if ( strchr( buffer, '\n' ) == NULL )` is an insufficient test for file's end. – chux - Reinstate Monica Dec 27 '20 at 05:30
  • @chux-ReinstateMonica: Good point. I believe I have fixed it now. – Andreas Wenzel Dec 27 '20 at 05:38
  • I like the fix. Should you care, an input line like "123 456xyz\n" passes even with junk after the 2nd `int`. OP likely does not care. – chux - Reinstate Monica Dec 27 '20 at 05:43
  • @chux-ReinstateMonica: Yes, that is why I pointed out the possibility of using `strtol` in my answer, as it allows stricter input validation. However, I did not add a code example of using it, as this additonal input validation would likely have confused the OP more than helping them. – Andreas Wenzel Dec 27 '20 at 06:00
  • @joy: I believe I now understand what you were trying to do with `ftell`. I have rewritten the start of my answer accordingly. – Andreas Wenzel Dec 27 '20 at 06:19
  • @joy: I have now also added an alternative solution which only uses `fscanf` instead of `sscanf`. That is probably the simplest solution to your problem, as it does most of the work for you, but it also is the least powerful solution in terms of input validation. – Andreas Wenzel Dec 27 '20 at 06:33
1

There are couple of problems in your code

  1. fread(buffer,size,*open_file);, buffer is not allocated

    A) you have to allocate memory using malloc or calloc if you use pointers and also free after you are done with it.
    If you want to avoid the headache of allocating and freeing, you better use an array sufficient enough to store the contents.

  2. fread takes 4 arguments

    A) 4th argument is not FILE , its FILE* , use only open_file not *open_file and you have not used nmemb(number of members) parameter
    On success, fread() return the number of items read, so check the return value to avoid errors.

  3. ftell() returns the current offset , Otherwise, -1 is returned
    A) you really don't need it, check the return value of fread to find out you have reached the EOF.

  4. check the syntax of sscanf and example

IrAM
  • 1,720
  • 5
  • 18
1

OP's code failed to seek the end of the array, allocate space for buffer and used fread() incorrectly.
Correct version shown with some error checking.

//char *buffer;
//int size = ftell(open_file); 
//int read_file = fread(buffer,size,*open_file);

// Seek and report, hopefully, the end position
if (fseek(open_file, 0, SEEK_END)) { fprintf(stderr, "fseek() failed.\n"); return EXIT_FAILURE;
long size = ftell(open_file); 
if (size == -1) { fprintf(stderr, "ftell() failed.\n"); return EXIT_FAILURE; }

// Allocate memory and read
if ((unsigned long) size > SIZE_MAX) { fprintf(stderr, "size too big.\n"); return EXIT_FAILURE; }
char *buffer = malloc((size_t) size);
if (buffer == NULL) { fprintf(stderr, "malloc() failed.\n"); return EXIT_FAILURE; }
size_t length = fread(buffer, sizeof *buffer, size, open_file);

// Use `length` as the number of char read
// ...
// when done
free(buffer);

Other problems too.

// while (size != EOF){
//    sscanf("%d%d",Integer1, Integer2);
// }

Maybe later, GTG.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256