How to read a file line-by-line in C?

Question

I have a text file with up to 100 IP addresses, 1 per line. I need to read each address, as a string, into an array called "list". First, I'm assuming that "list" will need to be a two-dimensional char array. Each IP address is 11 characters in length, 12 if you include '\0', so I declared list as follows:

char list[100][12];

Next, I attempt to use fgets to read the stream:

  for (i = 0; i < 100; i++)  
  {  
      if (feof(stream))  
          break;  
          for (j = 0; j < 12; j++)  
          fgets(&list[i][j], 12, stream);  
      count++;  
  }

To check to see if the strings were read properly, I attempt to output them:

  for (i = 0; i < 5; i++)  
  {  
      for (j = 0; j < 11; j++)  
          printf("%c", list[i][j]);  
      printf("\n");  
  }

After running the program, it's clear something is wrong. Being a beginner, I'm not sure what, but I'm guessing I'm reading the file wrong. There are no errors. It compiles, but prints a strange address on two lines.

Edit:

I replaced the fgets code with this:

for (i = 0; i < 100; i++)
  {
      if (feof(stream))
          break;
      fgets(list[i], 12, stream);
      count++;
  }

It now prints five strings, but they are "random" characters from memory.

Answer 1

The second loop is not necessary and it corrupts your memory. You should do something like this,

for (i = 0; i < 100; i++)
{
if (feof(stream))
break;
fgets(&list[i][j], 12, stream);
count++;
}

To check to see if the strings were read properly, I attempt to output them:

for (i = 0; i < 5; i++)
{
printf("%s\n", list[i]);
}

Answer 2

for (i = 0; i < 100; i++) {

   if (feof(fp))
       break;

   fscanf(fp,"%s\n",list[i]);

}

Answer 3

Do not use feof() as your loop condition; it will not return true until after you've tried to read past the end of the file, meaning your loop will execute one time too many. Check the result of your input call (whether you use fgets() or fscanf()) to see if it succeeded, then check feof() if you got an error condition.

if (fgets(buffer, sizeof buffer, stream) != NULL)
{
  // process the input buffer
}
else if (feof(stream)
{
  // handle end of file
}
else
{
  // handle read error other than EOF
}

fgets() reads entire strings, not individual characters, so you don't want to pass the address of each individual character in your string. Call it like so instead:

if (fgets(list[i], sizeof list[i], stream) != NULL)
{
  // process input address
}

And now, for Bode's usual spiel about arrays and pointers...

When an array expression appears in most contexts, the type of the expression is implicitly converted from "N-element array of T" to "pointer to T", and the value of the expression is the address of the first element of the array. The exceptions to this rule are when the array expression is the operand of the sizeof or & operators, or it is a string literal that is being used as an initializer in a declaration. When you hear people say "arrays and pointers are the same thing", they're garbling that rule. Arrays and pointers are completely different animals, but they can be used interchangeably in some contexts.

Note that in the code above I passed list[i] as the first argument to fgets() without any decoration (such as the & operator). Even though the type of list[i] is "12-element array of char", in this context it is implicitly converted to type "pointer to char", and the value will be the address of list[i][0]. Note that I also passed that same expression to the sizeof operator. In that case, the type of the array expression is not converted to a pointer type, and the sizeof operator returns the number of bytes in the array type (12).

Just to nail it down:

Expression      Type             Implicitly converted to
----------      ----             ----
list            char [100][12]   char (*)[12] (pointer to 12-element array of char)
list[i]         char [12]        char *
list[i][j]      char             N/A

What all this means is that fgets() will read up to the next 12 characters (provided it doesn't hit a newline or EOF first) and store it starting at list[i][0]. Note that fgets() will write a terminating nul character (0) to the end of your string. Note also that if fgets() encounters a newline and there's room in the target array for it and the terminating nul, fgets() will store the terminating newline before the nul character. So if your input file has a line like

1.1.1.1\n

then the contents of your input buffer after the read will be "1.1.1.1\n\0xxx" where x is some random value. If you don't want the newline there, you can use the strchr() function to find it and then overwrite it with a 0:

char *newline;
...
if ((newline = strchr(input[i], '\n')) != NULL)
{
  *newline = 0;
}

Since fgets() stops at the next newline, and since your input buffer is sized for 12 characters, it's possible for you to run into a situation where you have a newline as the next input character in the file; in that case, fgets() will write only that newline to the input buffer, so you'll have some empty entries, which is probably not what you want. You might want to add an extra byte to your input buffer in order to avoid that situation.

Putting it all together:

char list[100][13];
...
for (i = 0; i < 100; ++)
{
  if (fgets(list[i], sizeof list[i], stream) != NULL)
  {
    char *newline = strchr(list[i], '\n');
    if (newline != NULL)
      *newline = 0;
    printf("Read address \"%s\"\n", list[i]);
    count++;
  }
  else if (feof(stream))
  {
    printf("Reached end of file\n");
    break;
  }
  else
  {
    printf("Read error on input; aborting read loop\n");
    break;
  }
}

Answer 4

Your call to fgets reads up to 11 characters from the stream into the array. So you don't want to be calling that once for each character of each string.

Just think about those loops: with i=0 and j=0, it reads up to 11 characters to &list[0][0]. Then with i=0 and j=1, it reads another 11 characters to &list[0][1]. That's wrong for two reasons - it overwrites the result of the last call, and potentially it writes more bytes than list[0] can hold.

Answer 5

I wrote a function for reading lines. I think it should be safe.

Check : io_readline

https://github.com/arhuaco/junkcode/blob/master/junk/misc/atail.c

Answer 6

First, reading:

      for (j = 0; j < 12; j++)  
      fgets(&list[i][j], 12, stream);  

You have a big problem right here. This is attempting to read a string into each successive character in your array.

All in all, I think you're making this a lot more complex than it needs to be. Think of your array as 100 strings, and fgets will work with a string at a time. That means reading can look something like this:

for (i=0; i<100 && fgets(list[i], 11, string); i++)
    ;

There is one other minor detail to deal with: fgets() normally retains the new-line at the end of each line. As such, you may need to leave room for 13 characters (11 for address, 1 for new-line, 1 for NUL terminator), or else you may want to read the data into a temporary buffer, and copy it to your list only after you've stripped off the new-line.

In your current code for printing the strings, you're working one character at a time, which can work, but is unnecessarily difficult. Several people have suggested using the %s printf conversion, which is fine in itself. To go with it, however, you have to simplify your indexing a bit. Printing the first six addresses would look something like this:

for (i=0; i<6; i++)
    printf("%s", list[i]);