How to create an array of strings given an array of pointers?

Question

I am given a text file and I need to put it in a buffer and use get_lines to make an array of pointers after converting each line to a string. I am having trouble with just the get_lines function as I am getting a seg fault when run it.

Here's my code:

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

int readfile(FILE *fp, char **cbuf);       
char **get_lines(char *cbuf, int bufsize, int word);
int readword(FILE*tmp);

int main(int argc, char *argv[])
{
  int i,bufsize, num_word;
  char *cbuf;
  char **lines;
  FILE *fp;
  FILE *tmp;

  if( (fp=fopen(argv[1],"r")) == NULL)
    {
      perror("ERROR: bad/no filename");
      exit(0);
    }
  tmp = fopen(argv[1],"r");
  bufsize = readfile(fp,&cbuf);
  num_word = readword(tmp);
  lines = get_lines(cbuf, bufsize, num_word) ; 
  i=0;
  while( lines[i] != NULL) { 
    printf("%i\t%s\n",i,lines[i]); 
    i++;
    }
  return 0;
}

int readfile(FILE *fp,char**cbuf)
{
  int i;
  char c;

  fseek(fp, 0L, SEEK_END);
  int bufsize = ftell(fp);
  fseek(fp, 0L, SEEK_SET);

  *cbuf = (char *)malloc(sizeof(char) * bufsize);

  for (i = 0; i < bufsize; i++)
    {
      c = fgetc(fp);
      (*cbuf)[i] = c;
    }
  return bufsize;
}

int readword(FILE*tmp)
{
  int word = 0;
  char c;

  while((c = fgetc(tmp)) != EOF )
    {
      if (c == '\n')
      word++;
    }
  return word;
}

char **get_lines(char *cbuf, int bufsize, int word)
{
  int i = 0, j = 0, counter = 0;
  char (*lines)[bufsize];
  lines = (char**)malloc(sizeof(char*)*bufsize);

  counter = cbuf;  
  for (i = 0; i < bufsize; i++)
    {
      if(cbuf[i] == '\n')
    {
      cbuf[i] == '\0';
      counter = cbuf[i + 1];
      j++;
    }else
    {
      *lines[j] = &counter;
    }
    }

  lines[word] == NULL;
  return lines;
}

The violation causing the fault is not immediately obvious to me, can someone tell me what might be wrong in get_lines()?

Answer 1

This code is wrong:

char (*lines)[bufsize];
lines = (char**)malloc(sizeof(char*)*bufsize);

It allocates a pointer to an array of char. Then you malloc the wrong amount of space, cast to the wrong type, and write *lines[j] = &counter; which tries to store a pointer in a char.

You should get many compiler errors/warnings for the get_lines function. It's important to pay attention to such messages as they are telling you that something is wrong with your code. There's no point even starting to investigate a segfault until you have fixed all the errors and warnings.

See here for a great guide on how to debug your code; I suspect you would fail the rubber duckie test on the get_lines function.

---

Here is a fixed version (untested):

// Precondition: cbuf[bufsize] == '\0'
//
char **get_lines(char *cbuf, size_t bufsize, size_t num_lines)
{
    // +1 for the NULL termination of the list
    char **lines = malloc((num_lines + 1) * sizeof *lines);

    size_t line = 0;
    while ( line < num_lines )
    {
        lines[line++] = cbuf;
        cbuf = strchr(cbuf, '\n');

        if ( !cbuf )
            break;

        *cbuf++ = '\0';
    }

    lines[line] = NULL;
    return lines;
}

In your existing code there is no room to write the null terminator for the last line; my advice is to make readfile actually malloc one extra byte and make sure that is set to 0.