1

I want to make an array of strings where each string is one line from a text file. When i do strcpy(stringList[lineCount], lineBuf) I get segmentation fault and don't know why.

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

#define MAXLINELENGTH 20
#define MAXLINES 10

int fileToStringList(char* fileName, char** stringList){
    char* lineBuf = (char*) malloc(MAXLINELENGTH);
    size_t lineBufSize = 0;
    int lineCount = 0;
    ssize_t lineSize;
    FILE* fp = fopen(fileName, "r");

    if(!fp){
        printf("Error reading file");
        exit(1);    
    }

    lineSize = getline(&lineBuf, &lineBufSize, fp);
    while(lineSize >= 0){
        strcpy(stringList[lineCount], lineBuf);//segmentation fault
        lineCount++;
        lineSize = getline(&lineBuf, &lineBufSize, fp);
    }
    fclose(fp);
    return lineCount;
}
int main(int argc, char* argv[]){
    char* fileName = argv[2];
    char** stringList = (char**) malloc(MAXLINES);
    int lineCount;
    lineCount = fileToStringList(fileName, stringList);
    printf("Number of lines: %d\n", lineCount);
}
  • I can guess it's because of "(char**) malloc(MAXLINES)". Try to look here https://stackoverflow.com/questions/15686890/how-to-allocate-array-of-pointers-for-strings-by-malloc-in-c – nvf Sep 23 '22 at 10:58
  • 1
    The correct syntax for `malloc` is `Type *variable = malloc(NUMBER_OF_ELEMENTS * sizeof *variable);`, where `Type` is the type of the elements, in your case `char *` or `char`. – mch Sep 23 '22 at 11:23
  • 1
    The problem isn't so much the malloc itself as the fact that `stringList[n]` is not initialized. It should be set to point at allocated memory before calling strcpy. – Lundin Sep 23 '22 at 11:51
  • @Oscar Stenqvist, What should happen if the line is longer than `MAXLINELENGTH`? – chux - Reinstate Monica Sep 23 '22 at 12:00

1 Answers1

0

At least these issues:

Uninitialized value

stringList[lineCount] not yet assigned. strcpy() is receiving a junk destination pointer. Allocate first, use strdup or use lineBuf.

// strcpy(stringList[lineCount], lineBuf);
stringList[lineCount] = strdup(lineBuf);

// or "take" lineBuf
stringList[lineCount] = lineBuf;
lineBuf = NULL;
lineBufSize = 0;

Add limit

// while(lineSize >= 0){
while(lineSize >= 0 && lineCount < MAXLINES){

Unneeded allocation

getline() allocates as needed. Cast not needed. Likely off by 1 and MAXLINELENGTH + 1 was needed.

// char* lineBuf = (char*) malloc(MAXLINELENGTH);
char* lineBuf = NULL;

Missing free()

fileToStringList() fails to free lineBuf.

Wrong size and unnecessary cast

// char** stringList = (char**) malloc(MAXLINES);
char** stringList = malloc(sizeof stringList[0] * MAXLINES);

Example re-write:

for (lineCount = 0; lineCount < MAXLINES; lineCount++) { 
  stringList[lineCount] = NULL;
  size_t lineBufSize = 0;
  if (getline(&stringList[lineCount], &lineBufSize, fp)) < 0) {
    break;
  }
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256