For loop crashes my program?

Question

For whatever reason the for loop in getLetters causes a crash.

I haven't been able to single out a specific source within it, maybe someone could help?

I think I might have done my pointers incorrectly. How should I fix this?

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


int main() {
    FILE *ifp;
    ifp = fopen("dictionary.txt", "r");
    int* lValues[26];
    int* lDist[26];
    int* lUsed[26];
    int*fRun = 0;
    int dictLen;
    int i;
    int bestValue;
    fscanf(ifp, "%d", &dictLen);
    char bestWord[7];
    char dictionary[dictLen][7];
    char* letters[7];
    char* userWord[7];

    //reads the dictionary into the array
    for (i = 0; i < dictLen; i++) {
        fscanf(ifp, "%s", &dictionary[i]);
    }

    distribution();
    values();

    while (i != 2) {

        getLetters();

        printf("Welcome to the Scrabble Practice Program!\n");
        printf("\nHere are your letters: %s \n", letters);
        printf("\nWhat would you like to do?\n");
        printf("\t1-Enter Word\n");
        printf("\t2-Quit\n\n");
        printf("User Selection:");
        scanf("%d", &i);
    }

    //fclose(ifp); not sure why, but this crashes the program

    return 0;
}

//Gets the user's set of letters
void getLetters() {
    char* letters[7];
    int* lDist[26];
    int* lUsed[26];
    int lCur;
    int i;

    srand(time(0));

    for (i = 0; i < 7; i++) {

        *letters[i] = 65 + rand()%26;
        lCur = (int)*letters[i] - 65;

        while (*lUsed[lCur] >= *lDist[lCur]){

            *letters[i] = 65 + rand()%26;
            lCur = (int)*letters[i] - 65;
        }

        *lUsed[lCur]++;
    }
}

//Sets the number of each letter available for distribution
void distribution() {
    int* lDist;

    //Number for A
    lDist[0] = 9;

    //Number for B
    lDist[1] = 2;

    //Number for C
    lDist[2] = 2;

    //Number for D
    lDist[3] = 4;

    //Number for E
    lDist[4] = 12;

    //Number for F
    lDist[5] = 2;

    //Number for G
    lDist[6] = 3;

    //Number for H
    lDist[7] = 2;

    //Number for I
    lDist[8] = 9;

    //Number for J
    lDist[9] = 1;

    //Number for K
    lDist[10] = 1;

    //Number for L
    lDist[11] = 4;

    //Number for M
    lDist[12] = 2;

    //Number for N
    lDist[13] = 6;

    //Number for O
    lDist[14] = 8;

    //Number for P
    lDist[15] = 2;

    //Number for Q
    lDist[16] = 1;

    //Number for R
    lDist[17] = 6;

    //Number for S
    lDist[18] = 4;

    //Number for T
    lDist[19] = 6;

    //Number for U
    lDist[20] = 4;

    //Number for V
    lDist[21] = 2;

    //Number for W
    lDist[22] = 2;

    //Number for X
    lDist[23] = 1;

    //Number for Y
    lDist[24] = 2;

    //Number for Z
    lDist[25] = 1;
}


//Sets the value of all of the letters
void values() {

    int* lValues;

    //Value of A
    lValues[0] = 1;

    //Value of B
    lValues[1] = 3;

    //Value of C
    lValues[2] = 3;

    //Value of D
    lValues[3] = 2;

    //Value of E
    lValues[4] = 1;

    //Value of F
    lValues[5] = 4;

    //Value of G
    lValues[6] = 2;

    //Value of H
    lValues[7] = 4;

    //Value of I
    lValues[8] = 1;

    //Value of J
    lValues[9] = 8;

    //Value of K
    lValues[10] = 5;

    //Value of L
    lValues[11] = 1;

    //Value of M
    lValues[12] = 3;

    //Value of N
    lValues[13] = 1;

    //Value of O
    lValues[14] = 1;

    //Value of P
    lValues[15] = 3;

    //Value of Q
    lValues[16] = 10;

    //Value of R
    lValues[17] = 1;

    //Value of S
    lValues[18] = 1;

    //Value of T
    lValues[19] = 1;

    //Value of U
    lValues[20] = 1;

    //Value of V
    lValues[21] = 4;

    //Value of W
    lValues[22] = 4;

    //Value of X
    lValues[23] = 8;

    //Value of Y
    lValues[24] = 4;

    //Value of Z
    lValues[25] = 10;
}

Answer 1

Every single array access in your getLetters function can cause a problem: you declare three arrays of pointers that are never initialized, so any time you dereference an element of any of the arrays you are dereferencing a garbage memory address, and sometimes trying to write to that memory address.

From your code it's unclear why you need arrays of pointers.

Why not simply:

char letters[7];
int lDist[26];
int lUsed[26];

And get rid of the dereferencing?

But even then, the function has problems. You'll need to initialize your array values, at least lDist and lUsed. You compare against elements of lDist but otherwise never change those values.

Answer 2

It does not appear that the above code does what you intend it to do. When you declare the variable

char* letters[7];

… you're declaring an array of 7 char *s, not 7 chars. Thus, when you say

*letters[i] = 65 + rand()%26;

… you first index into the array and retrieve the pointer at index i. You then dereference that pointer and try to store a character there. The problem is that letters is never initialized, and is most likely filled with junk: for any value i, letters[i] will return a random value. Thus, when you try to dereference that as an address, you are indexing into someone else's memory, and writing to it will cause a segmentation fault.

It seems that you meant to declare letters (and lDist and lUsed) like so:

char letters[7];
int lDist[26];
int lUsed[26];

That means an array of chars with length 7. You would then address them without dereferencing:

letters[i] = 65 + rand()%26;

You could also do the above as…

*(letters + i) = 65 + rand()%26;

… but you generally don't mean to mix the two together.

Answer 3

*letters[i] = 65 + rand()%26;

letter[i] refers nowhere meaningful(uninitialized) and you can end up with Access violation fault.

Further

char* letters[7];
int* lDist[26];
int* lUsed[26];

all of them are arrays of pointer and not the array of basic types(ie. int,char etc.).I think you probably wanted

char letters[7];//Array of seven characters
int lDist[26];//Array of 26 ints
int lUsed[26];//Array of 26 ints

and start referring *letters[i] simply as letters[i]

Answer 4

No need to use pointers, use plain arrays will do, like this (I also added initialization for lUsed and lDist):

void getLetters() {
    char letters[7];
    int lDist[26];
    int lUsed[26];
    int lCur;
    int i;


    srand(time(0));

    // initialize the arrays
    for (i=0;i<26;i++) {
        lUsed[i] = lDist[i] = 0;
    }

    for (i = 0; i < 7; i++) {

        letters[i] = 65 + rand()%26;
        lCur = (int)letters[i] - 65;

        while (lUsed[lCur] >= lDist[lCur]){

            letters[i] = 65 + rand()%26;
            lCur = (int)letters[i] - 65;

        }

        lUsed[lCur]++;

    }
}

Answer 5

letters, lDist and lUsed are arrays of pointers (which aren't initialized). Why don't you use arrays instead?

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

void getLetters(void)
{
    char letters[7];
    int lDist[26];
    int lUsed[26];
    int lCur;
    int i;

    srand(time(NULL));

    for (i = 0; i < 7; i++) {
        letters[i] = 65 + rand() % 26;
        lCur = (int) letters[i] - 65;

        while (lUsed[lCur] >= lDist[lCur]) {
            letters[i] = 65 + rand() % 26;
            lCur = (int) letters[i] - 65;
        }

        lUsed[lCur]++;
    }
}