Shuffle array (seg fault)

Question

So I am supposed to create a function that accomplishes: Purpose: program to shuffle the lines of a text file

• Read the file into an array
• Count lines and maximum length
• Compute maximum width for array
• Get file pointer to the beginning
• Reserve memory for a dynamic array of strings
• Read a line and store in allocated memory
• Turn the \n into \0
• Print lines from array (test)
• Shuffle array
• Print lines from array (test)
• Free memory and close file

(just to give some background)

However, when I go to print the shuffled array, I get segmentation faults. Occasionally, it prints one or two of the strings, but sometimes It just says "Shuffled Array" and then I get a segmentation fault. Any ideas?

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

// Accepts: command line input
// Returns: 0 if no error

int main(int argc, char *argv[] ){
    int x = 0, i, lineCount = 0, maxLen = 0;
    char line[500], temp;
    FILE *file = fopen( argv[1], "r" );
//  check if file exists
    if (file == NULL){
        printf("Cannot open file\n");
        return 1;
    }
//  Gets lines, max length of string    
    while (fgets(line, sizeof(line), file) != NULL){
        lineCount++;
        if (strlen(line) > maxLen)
            maxLen = strlen(line);
    }
    rewind(file);
    char *lineArray[lineCount];
    while (fgets(line, sizeof(line), file) != NULL) {
            lineArray[x] = malloc(strlen(line));
        if (lineArray[x] == NULL){
            printf("A memory error occurred.\n");
            return(1);
        }
            strcpy(lineArray[x], line);
//  change \n to \0
        lineArray[x][strlen(lineArray[x])-1] = '\0';
        x++;
    }
    printf("File %s has %d lines with maximum length of %d characters\n",
        argv[1], lineCount, maxLen);
    printf("Original Array\n");
    for (x = 0; x < lineCount; x++)
        printf("%2d %s\n", x, lineArray[x]);
//  Shuffle array
    srand( (unsigned int) time(NULL));
    for (x = lineCount - 1; x >= 0; x--){
        i = (int) rand() % lineCount;
        temp = lineArray[x];
        lineArray[x] = lineArray[i];
        lineArray[i] = temp;
    }
    printf("\nShuffled Array\n");
    for (x = 0; x < lineCount; x++)
        printf("%2d %s\n", x, lineArray[x]);
//  free allocated memory
    for (x = 0; x < lineCount; x++)
        free(lineArray[x]);
    free(lineArray);
    fclose(file);
    return 0;
}

Answer 1

The output from running cc on my machine makes the error pretty obvious.

$ cc tmp.c -o tmp
tmp.c:46:14: warning: incompatible pointer to integer conversion assigning to
      'char' from 'char *'; dereference with * [-Wint-conversion]
        temp = lineArray[x];
             ^ ~~~~~~~~~~~~
               *
tmp.c:48:22: warning: incompatible integer to pointer conversion assigning to
      'char *' from 'char'; take the address with & [-Wint-conversion]
        lineArray[i] = temp;
                     ^ ~~~~
                       &
2 warnings generated.

You need to fix your variables, you can't use a char where you intend to use a char *.

Sorry, to make it more clear:

char line[500], temp;

should be:

 char line[500], *temp;

If you want clarification on why this is, let me know.

Lastly, it is not C-style (unless you are writing embedded C) to declare variables at the top of a method. Declare them as close to the usage point as possible. It makes it easier to find what you declared. For example, temp could be declared right above the loop where its used, or even better, in the loop itself.

Oh, and :

$ cc --version
Apple LLVM version 5.0 (clang-500.2.76) (based on LLVM 3.3svn)
Target: x86_64-apple-darwin13.0.0
Thread model: posix

Answer 2

What are you doing here?

free(lineArray);

lineArray is an array, defined as so:

char *lineArray[lineCount];

You shouldn't try to free() it, because you didn't malloc() it in the first place.

On top of that, this:

char line[500], temp;

should be this:

char line[500], *temp;

as your compiler should be trying to tell you.

There may be other problems, but since you choose not to provide your input file or to provide a version of your program that doesn't need one, nobody can compile and run it to find out.

EDIT: Making the above changes, changing lineArray[x] = malloc(strlen(line)); to lineArray[x] = malloc(strlen(line) + 1); as suggested by another answer, and using a suitable input file, I get the following output:

paul@local:~/src/c/scratch$ ./sta testfile
File testfile has 4 lines with maximum length of 13 characters
Original Array
 0 john doe
 1 jane fish
 2 donny brutus
 3 fishy mcgee

Shuffled Array
 0 john doe
 1 jane fish
 2 fishy mcgee
 3 donny brutus
paul@local:~/src/c/scratch$

You seriously need to learn to use your compiler better, though. You should be invoking gcc like this:

gcc -o myprog myfile.c -std=c99 -pedantic -Wall -Wextra

to get all the most useful warnings.