CODEWITHSUNDEEP
cgetline
Student
Student

Reputation: 719

Saving getline() output to an external array

The external array srclns should keep each read line from a text file. But reading it's content afterwards seems like read lines are empty strings. What am I missing in the code below?

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

#define MAXSRC 20


char *srclns[MAXSRC];   /* source lines */

size_t read_lines(char *path)
{
  FILE *stream;
  ssize_t read;
  char *lnptr;
  size_t n;
  size_t count;
  stream = fopen(path, "r");
  lnptr = NULL;
  n = 0;
  count = 0;
  if (!stream) {
    fprintf(stderr, "Can't open source '%s'\n", path);
    exit(EXIT_FAILURE);
  }
  while ((read = getline(&lnptr, &n, stream)) != -1) {
    srclns[count++] = lnptr;
  }
  free(lnptr);
  fclose(stream);
  return count;
}


int main()
{

  size_t n = read_lines("foo.txt");
  for (size_t i = 0; i<n; i++)
    printf("%zu %s\n", i, srclns[i]);
  exit(EXIT_SUCCESS);
}

This prints only the line numbers with seemingly empty strings afterwards:

0 
1 
2 
3 
4 
5

Upvotes: 2

Views: 423

Answers (2)

LNiederha
LNiederha

Reputation: 948

So from what I can see not only does your program not work but it might have memory leaks. This is due to the behavior of getline which uses dynamic allocation.

Let's take a closer look at what your program does, in particular the while ((read = getline(&lnptr, &n, stream)) != -1) loop:

getline will work with &lnptr which is of type char**.

  • If the pointer is NULL it will allocate enough memory on heap (dynamic) to store the line that is being read.
  • If the pointer is not NULL then it is expected to point on a buffer of size n
    • If the buffer is big enough (greater or equal to the line length) it is used to store the string.
    • If the buffer is too small then memory is reallocated by getline so there is a big enough buffer available. Upon reallocation, n is updated to the new buffer size. And in certain cases reallocation will imply that lnptr has to be modified and will be. (This might happen if there is not enough consecutive memory free riIght after the current buffer. In that case the memory will be allocated somewhere else on heap. If this is of interest to you I suggest you research is because dynamic memory allocation is a rather complex topic, else just know the pointer might change, that's enough for now).

Now here are the issues with your program (at least this is what I can infer from the information I have. I might be wrong but this seems the most plausible interpretation):

  • On the first iteration of the loop lnptr is NULL. Thus getline allocates memory on heap and stores the line, and update lnptr to point on the newly allocated buffer.
  • Within the loop you store the pointer to the allocated buffer in srclns[0]
  • On the subsequent iterations the buffer is overwritten and maybe resized by getline, and you still store the pointer to the same buffer srclns[count].
  • After the loop you free the buffer and discard the memory every pointer in srclns points to.
  • When you print you most likely read an invalid memory zone (which is the zone pointed by the pointer you just freed) and luckily it seems to start with an termination character (Last line of your file was probably an empty line and nothing actively changed this memory zone after the free...)

How to fix it: You could explicitly handle dynamic allocation with malloc and/or calloc but that seem a bit complicated and, as shown before, getline can handle it for you. My suggestion is as follow:

  1. Set all your elements in srclns to NULL 
    for(int i = 0; i < MAXSRC; ++i)
{
   srclns[i] = NULL;
}
  2. Then rework the while loop to pass a new element of srclns in each iteration. Each call to getline will see an NULL pointer, thus allocating memory and updating the cell of srclns to point on it. Bonus with this implementation your certain of never going out of bounds of srclns: 
    for(int i = 0; i < MAXSRC; ++i)
{
    n = 0
    if(getline(&srclns[i], &n, stream) == -1)
    {
        break;  // We get out if we reached OEF
    }
}
  3. Free all of this allocated memory in main after you accessed it for your printf 
    for(int i = 0; i < MAXSRC; ++i)
{
  if(srclns[i] != NULL)
  {
     free(srclns[i]);
  }
}
  4. Adjust. I did no test on the code so I might have made some mistakes... feel free to correct it. You might also want to adjust the code to match your needs.

Upvotes: 3

Yun
Yun

Reputation: 3812

The function getline will only allocate memory if lnptr is NULL (ref). This is the case for the first iteration, but it will need to be reset to NULL afterwards:

while ((read = getline(&lnptr, &n, stream)) != -1) {
  srclns[count++] = lnptr;
  lnptr = NULL;
}

Otherwise, lnptr will still point to the memory allocated in the first iteration for all subsequent iterations and getline will repeatedly try to write to that location.

Even though it is not the cause of the problem, the allocated memory should be free'd. For example, by adding these lines before exit(EXIT_SUCCESS):

for (size_t i = 0; i<n; i++)
  free(srclns[i]);

Whether or not using getline is a good practice is another discussion which you may want to look into. It is not the most portable solution.

Upvotes: 2

Related Questions