2

I've looked around and haven't seen this question answered yet. Basically I am trying to create an array of integers from text files that have sequences of integers e.g, 2 5 2 9 1 0 3 53 7 . I want to print an error message if line in the text file exceed 10 integers. There is only one line in the text file.

Here is my code so far:

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

int main()
{
    FILE *file = fopen("somenumbers.txt", "r");
    int integers[10];


    int i=0;
    int num;

    if (file == NULL)
    {
        printf("Error Reading File\n");
        exit (0);
    }

    while(fscanf(file, "%d", &num) > 0) {
        integers[i] = num;
        i++;
    }

    for (i = 0; i < 16; i++)
    {
        printf("Number is: %d\n\n", integers[i]);
    }

    fclose(file);

    return 0;
}

Should I check the check the contents of the array after it is finished being created or during the initial iteration through the line? Are there any functions that would make it easy to determine if the line in the text file is larger than the limit(10)?

ilim
  • 4,477
  • 7
  • 27
  • 46
  • Your code is vulnerable to a buffer-overflow exploit because you never check to see if more than 10 numbers have been read from the file. – Dai Feb 06 '17 at 08:26
  • `int counter = 0; while(fscanf(file, "%d", &num) > 0) { if(i < 10) integers[i++] = num; counter++; } if(counter > 10) printf("too many\n");` – BLUEPIXY Feb 06 '17 at 08:29
  • Why not `while(fscanf(file, "%d", &num) == 1)`? `fscanf()` returns number of values read. – RoadRunner Feb 06 '17 at 09:27

6 Answers6

2

You must check in while loop as below;

while(fscanf(file, "%d", &num) > 0) {
    if (i >= 10) {
        printf("error\n");
        break;
    }
    integers[i++] = num;
}
avatli
  • 610
  • 6
  • 16
1

You should ensure that you never access integers[10], otherwise it's array out-of-bounds error which results in undefined behavior (i.e. literally anything can happen after that). So if you succeeded in reading 11-th number (which should go into integers[10]), you should stop the loop immediately.

yeputons
  • 8,478
  • 34
  • 67
1

The reason you are getting the error is the size of integers array being 10. Due to that size, if you read more than 10 integers, you will have a segment violation problem.

To find out that you have more than 10 integers, all you need to understand you should give an error is to read the 11th integer. So instead of declaring the array with size 10, switch it to 11. Then, when you read the 11th integer you may print an error message and exit properly.

Also, you may want to bound the loop printing the numbers by the amount of integers you have read.

Below is a sample code, based on yours, that implements the fixes I mentioned.

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

int main()
{
    FILE *file = fopen("somenumbers.txt", "r");
    int integers[11];

    int i=0, k=0;
    int num;

    if (file == NULL)
    {
        printf("Error Reading File\n");
        exit (0);
    }

    while(fscanf(file, "%d", &num) > 0) {
        integers[i] = num;
        if(k++ == 10) {
        {
            printf("Too many integers!!!\n"); /* or any other error message you'd like */
            exit (0);
        }
    }

    /* loop iterates until k integers are printed. k contains the # of integers read. */
    for (i = 0; i < k; i++)
    {
        printf("Number is: %d\n\n", integers[i]);
    }

    fclose(file);
    return 0;
}
ilim
  • 4,477
  • 7
  • 27
  • 46
0

Check before:

...
while (fscanf(file, "%d", &num) > 0) {
        if (i >= 10) {
                /* handle error */
                break;    /* or return */
        }
        ...

to prevent trying to access an array element that does not exist

John Hascall
  • 9,176
  • 6
  • 48
  • 72
0

You have two errors:

1) When reading, you may write the input value outside the array boundary

2) When printing, you for sure acces outside array boundary.

Try this instead:

while(fscanf(file, "%d", &num) > 0) {
    integers[i] = num;
    i++;
    if (i == 10)
    {
        break;  // Can't store more value so stop the loop using break
    }
}

// Save the number of values read
int total = i;
for (i = 0; i < total; i++)
               // ^^^^ notice
{
    printf("Number is: %d\n\n", integers[i]);
}

As an alternative to break you can put the check of i into the while condition like:

while(i < 10 && fscanf(file, "%d", &num) > 0) {
    //^^^^^^ notice

    integers[i] = num;
    i++;
}
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
0

You have some issues with your code:

  • The code posted is prone to buffer overflow, as you are not checking if more than 10 integers have been found. This means you will be accessing outside the bounds of integers[10], which only causes undefined behavour.
  • Since you want to read one integer at a time with fscanf(), you should use:

    while (fscanf(file, "%d", &num) == 1)
    

    Instead of:

    while(fscanf(file, "%d", &num) > 0)
    

    fscanf() returns the number of values read, and using 1 instead of > 0 would make more sense in this case.

  • This segment here:

    for (i = 0; i < 16; i++)
    {
        printf("Number is: %d\n\n", integers[i]);
    }
    

    is accessing beyond bounds of integers[10]. You need to change the guard so you don't exceed the limit of 10 integers.

Your code can look like this:

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

#define MAXINT 10

int main(void) {
    FILE *file;
    int integers[MAXINT], num;
    size_t count = 0;

    file = fopen("somenumbers.txt", "r");
    if (!file) {
        fprintf(stderr, "%s\n", "Error reading file");
        exit(EXIT_FAILURE);
    }

    while (fscanf(file, "%d", &num) == 1) {
        if (count == MAXINT) {
            printf("More than %d integers found!\n", MAXINT);
            exit(EXIT_FAILURE);
        }
        integers[count++] = num;
    }

    printf("Success! No more than %d integers found:\n", MAXINT);
    for (size_t i = 0; i < count; i++) {
        printf("integers[%zu] = %d\n", i, integers[i]);
    }

    fclose(file);

    return 0;
}
Community
  • 1
  • 1
RoadRunner
  • 25,803
  • 6
  • 42
  • 75