Why is this C code giving me a bus error?

Question

I have, as usual, been reading quite a few posts on here. I found a particular useful posts on bus errors in general, see here. My problem is that I cannot understand why my particular code is giving me an error.

My code is an attempt to teach myself C. It's a modification of a game I made when I learned Java. The goal in my game is to take a huge 5049 x 1 text file of words. Randomly pick a word, jumble it and try to guess it. I know how to do all of that. So anyway, each line of the text file contains a word like:

   5049
   must
   lean 
   better 
   program 
   now
   ...

So, I created an string array in C, tried to read this string array and put it into C. I didn't do anything else. Once I get the file into C, the rest should be easy. Weirder yet is that it complies. My problem comes when I run it with ./blah command.

The error I get is simple. It says:

zsh: bus error ./blah

My code is below. I suspect it might have to do with memory or overflowing the buffer, but that's completely unscientific and a gut feeling. So my question is simple, why is this C code giving me this bus error msg?

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

//Preprocessed Functions 
void jumblegame();
void readFile(char* [], int);


int main(int argc, char* argv[])
{
    jumblegame();

}

void jumblegame()
{
    //Load File 
        int x = 5049; //Rows
        int y = 256; //Colums
        char* words[x]; 
        readFile(words,x);

    //Define score variables 
        int totalScore = 0;
        int currentScore = 0; 

   //Repeatedly pick a random work, randomly jumble it, and let the user guess what it is

}

void readFile(char* array[5049], int x) 
{
    char line[256]; //This is to to grab each string in the file and put it in a line. 
    FILE *file;
    file = fopen("words.txt","r");

    //Check to make sure file can open 
    if(file == NULL)
    {
        printf("Error: File does not open.");
        exit(1);
    }
    //Otherwise, read file into array  
    else
    {
        while(!feof(file))//The file will loop until end of file
        {
           if((fgets(line,256,file))!= NULL)//If the line isn't empty
           {
               array[x] = fgets(line,256,file);//store string in line x of array 
               x++; //Increment to the next line 
           }    
        }
    }

}

Answer 1

This line has a few problems:

array[x] = fgets(line,256,file);//store string in line x of array 

• You've already read the line in the condition of the immediately preceding if statement: the current line that you want to operate on is already in the buffer and now you use fgets to get the next line.

• You're trying to assign to the same array slot each time: instead you'll want to keep a separate variable for the array index that increments each time through the loop.

• Finally, you're trying to copy the strings using =. This will only copy references, it won't make a new copy of the string. So each element of the array will point to the same buffer: line, which will go out of scope and become invalid when your function exits. To populate your array with the strings, you need to make a copy of each one for the array: allocate space for each new string using malloc, then use strncpy to copy each line into your new string. Alternately, if you can use strdup, it will take care of allocating the space for you.

But I suspect that this is the cause of your bus error: you're passing in the array size as x, and in your loop, you're assigning to array[x]. The problem with this is that array[x] doesn't belong to the array, the array only has useable indices of 0 to (x - 1).

Answer 2

I suspect the problem is with (fgets(line,256,file))!=NULL). A better way to read a file is with fread() (see http://www.cplusplus.com/reference/clibrary/cstdio/fread/). Specify the FILE* (a file stream in C), the size of the buffer, and the buffer. The routine returns the number of bytes read. If the return value is zero, then the EOF has been reached.

char buff [256]; 
fread (file, sizeof(char), 256, buff); 

Answer 3

You are passing the value 5049 for x. The first time that the line

array[x] = ... 

executes, it's accessing an array location that does not exist.

It looks like you are learning C. Great! A skill you need to master early is basic debugger use. In this case, if you compile your program with

gcc -g myprogram.c -o myprogram

and then run it with

gdb ./myprogram

(I am assuming Linux), you will get a stack dump that shows the line where bus error occurred. This should be enough to help you figure out the error yourself, which in the long run is much better than asking others.

There are many other ways a debugger is useful, but this is high on the list. It gives you a window into your running program.

Answer 4

You are storing the lines in the line buffer, which is defined inside the readFile function, and storing pointers to it in the arary. There are two problems with that: you are overwriting the value everytime a new string is read and the buffer is in the stack, and is invalid once the function returns.

Answer 5

You have at least a few problems:

• array[x] = fgets(line,256,file)

  This stores the address of line into each array element. line in no longer valid when readFile() returns, so you'll have an array of of useless pointers. Even if line had a longer lifetime, it wouldn't be useful to have all your array elements having the same pointer (they'd each just point to whatever happened to be written in the buffer last)

• while(!feof(file))

  This is an antipattern for reading a file. See http://c-faq.com/stdio/feof.html and "Using feof() incorrectly". This antipattern is likely responsible for your program looping more than you might expect when reading the file.

• you allocate the array to hold 5049 pointers, but you simply read however much is in the file - there's no checking for whether or not you read the expected number or to prevent reading too many. You should think about allocating the array dynamically as you read the file or have a mechanism to ensure you read the right amount of data (not too little and not too much) and handle the error when it's not right.