C - Reading line by line from a file - wrong output displayed

Question

I am trying to read integers from a file one by one, and read it into the program as a row & column number.

My file contents are:

And my code:

fgets(line, 2, fp);
livecells = atoi(line);
fprintf(stderr, "\n%i live cells\n", livecells);    

while ( !feof(fp) ) 
{
  fgets(line, 5, fp);

  alive_row = atoi(&line[0]);
  alive_column = atoi(&line[2]);

  fprintf(stderr, "\n Cell: (%i)(%i)", alive_row, alive_column);       
}

where line is the character array (string) where the fgets contents are stored, and alive_row, & alive_column are integers.

But for some reason the output is:

3 live cells

 Cell: (0)(0)
 Cell: (2)(1)
 Cell: (2)(2)
 Cell: (2)(3)
 Cell: (2)(3)

The problem is, why is the first cell printed out (0)(0)? I don't know why this is being printed out... Apologies if it is blindingly obvious...

EDIT: And obviously the repeated cell (2)(3) at the end, oops.

The first column is printed out as `(0, 0)`, because `atoi` fails and it returns 0. Please don't use `atoi`, but rather use `strtol` which will make you enable to check if the conversion was successful or not. — Nemanja Boric, Sep 14 '13 at 09:58

Nemanja Boric · Accepted Answer · 2013-09-14T10:39:15.640

The first column is printed out as (0, 0), because atoi fails. Please don't use atoi, but rather use strtol which will make you enable to check if the conversion was successful or not.

The reason why atoi fails is because you have an extra \n character, because with fgets(fgets(line, 2, fp); you're reading only one character - because you passed 2 for size of buffer, and 1 element of the buffer is reserved for the \0 character. Just use enough big buffer to read the entire line, and pass the size of the buffer there.

In order to fix the other error, just don't use feof. Rather check the fgets return value to see how many characters it read from the file (or, if you really want to use feof make that check after the fgets call).

while (  fgets(line, sizeof(line), fp) > 0) 
{
  char* end;

  alive_row = strtol(&line[0], &end, 10);

  // If the end points to the &line[0], then no 
  // characters were converted!     
  if(end == &line[0])
  {
      printf("strtol failed!\n");
      return 0;
  }

  alive_column = strtol(&line[2], &end, 10);

  // If the end points to the &line[2], then no 
  // characters were converted!     
  if(end == &line[2])
  {
      printf("strtol failed!\n");
      return 0;
  }

  fprintf(stderr, "\n Cell: (%i)(%i)", alive_row, alive_column);       
}

We have a check &line[0] == end because the second argument passed to strtol is

Reference to an object of type char*, whose value is set by the function to the next character in str after the numerical value.

If there was no numerical value in the string, this pointer will point to the beginning of the string you've tried to convert.

If you don't know if the number of the digits will be 1 or 2, or any, you still can use strtol to help you here:

while (  fgets(line, sizeof(line), fp) > 0) 
{
  char* end;
  char* tmp;

  alive_row = strtol(&line[0], &end, 10);

  // If the end points to the &line[0], then no 
  // characters were converted!     
  if(end == &line[0])
  {
      printf("strtol failed!\n");
      return 0;
  }

  // If the previous strtol converted the first number
  // end will point to the space character (the first character after the number).
  // So, just pass
  // end + 1 which should point to the second number
  tmp = end + 1;
  alive_column = strtol(tmp, &end, 10);

  // If the end points to the tmp, then no 
  // characters were converted!     
  // (this is why we used this tmp to place end + 1 - we need to 
  //  check for the success of the strtol).
  if(end == tmp)
  {
      printf("strtol failed!\n");
      return 0;
  }

  fprintf(stderr, "\n Cell: (%i)(%i)", alive_row, alive_column);       
}

Ok, wont use atoi, have used strtol: alive_row = strtol(&line[0], NULL, 10); alive_column = strtol(&line[2], NULL, 10); but the output remains the same? — George Loman, Sep 14 '13 at 10:11
In order to check for the actual error, you need the second argument, and not the NULL. The second argument will point to the unparsed remaining part of the string. The strtol will indicate failure if that pointer is the same as the start pointer you've passed to it (if no characters were converted). — Nemanja Boric, Sep 14 '13 at 10:14
Still confused about this, sorry. So what are you saying the NULL need to be changed to? — George Loman, Sep 14 '13 at 10:24
I understand it a lot better now, thank you. But unfortunately I just remembered that all the integers in the text file can be up to 2 digits long, so I will probably have to start from scratch :/ — George Loman, Sep 14 '13 at 10:35
If you use `strtol`, then you can just use `end + 1` pointer to pass the next number to the next function call. It would work for any number of digits (even two). — Nemanja Boric, Sep 14 '13 at 10:36
Ok, so your code seems to make sense, however what is there to stop if from failing all the time? Because it always outputs strtol failed... — George Loman, Sep 14 '13 at 10:54
@GeorgeLoman You still need to fix the first `fgets`: use `fgets(line, sizeof(line), fp) ` instead of `fgets(line, 2, fp)` as it is told in the first part of the answer. — Nemanja Boric, Sep 14 '13 at 10:56

Gangadhar · Answer 2 · 2013-09-14T11:19:38.057

Since you are dealing withj single digits you simly use line[0]-'0' to convert line[0] into integer. while scaning first time just scan 5 bytes so that file stream move to next line.

   #include<stdio.h>

    int main()
    {

    FILE *fp;
    int alive_row;
    int alive_column,livecells;


    char line[10];
    fp=fopen("file1","r");
    fgets(line,5 , fp);
    livecells = line[0]-'0';
    fprintf(stderr, "\n %i live cells\n", livecells);

    while ( fgets(line, 5, fp)!=NULL)
    {
    //  puts(line);
      alive_row = line[0]-'0';
      alive_column = line[2]-'0';

      fprintf(stderr,"\n Cell: (%i)(%i)\n", alive_row, alive_column);
    }
    fclose(fp);
    }

With this you can split lines into two values as rows and columns.

file1:

CODE:

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

int main()
{

    FILE *fp;
    int alive_row=0;    
    int alive_column=0,livecells;
    int column=0,i;

    char line[10];
    fp=fopen("file1","r");
    fgets(line,5, fp);       //read first line
    livecells = atoi(line); //convert into int
    fprintf(stderr, "\n %i live cells\n", livecells);

    while ( fgets(line, 12, fp)!=NULL)      //read next lines
        {
        for(i=0;i<strlen(line)-1;i++)      //loop to separate rows and columns , here if space occurs make flag column as 1
            {
            if(line[i]!=' ' && column!=1 )     
            alive_row = 10*alive_row+line[i]-'0'; //convert into integers
            else
                {
                if(line[i]==' ')
                i=i+1;
                alive_column=10*alive_column+line[i]-'0';
                column=1; //you can write this statement just above if but you need to add two brace 
                }
            }

        printf(" Cell:(%d)(%d)\n\n", alive_row, alive_column);
        alive_row=0;   //after every iteration just make these value to default. 
        alive_column=0;
        column=0;
        }
    fclose(fp);
}

OUTPUT

 12 live cells

 Cell:(2)(1)

 Cell:(2)(2)

 Cell:(2)(3)

 Cell:(50)(5)

 Cell:(30)(20)

 Cell:(5)(30)

 Cell:(30)(330)

 Cell:(3390)(447)

 Cell:(12)(1234)

 Cell:(0)(0)

 Cell:(1234)(1)

 Cell:(154)(0)

Sorry, forgot to mention its not only single digits, its up to 2 digits for each row/col number. — George Loman, Sep 14 '13 at 10:15
Read 10 bytes with the fgets and then till space move to a alive_row and next alive_column ... line="55 21" and **alive_row = 10*(line[0]-'0') +(line[1]-'0')** and **alive_column = 10*(line[3]-'0') +(line[4]-'0')** or else just use strtok to spilt with the delimiter space and then strtol to convert into rows and columns — Gangadhar, Sep 14 '13 at 10:22
@GeorgeLoman Updated answer , in order to work with the variable number of digits — Gangadhar, Sep 14 '13 at 11:20

score 0 · Answer 3 · answered Sep 14 '13 at 10:04

fgets() reads the \n character, if possible.
If not, the \n will be read in the next reading operation.

Since your call is fgets(line, 2, fp);, only 1 character is read, and the 2nd is filled with a null character. So: line[0] == '3' and line[1] == '\0'.

The end-of-line will be read later, so your first reading operation, inside the while() loop is just this: "\n". This produces arbitrary results with atoi (zeros in your case).

Take in account that:

fgets(line, N, fp) reads, from the file fp, at most N - 1 characters from stdin, including \n, if possible. The result is put in line, and ALWAYS makes room at the end for a \0 character.

C - Reading line by line from a file - wrong output displayed

3 Answers3