2d Char arrays Partially overwriting Previous Entries

[TreeBranches]

My program begins by opening a dictionary file, the first line of which contains the number of words in the dictionary. If I print the words immediately after scanning them into the arrays, they appear fine, but if I later examine a previous entry, I can see it has been partially overwritten, usually after the first four letters. I think it's my mistake lies in my memory allocation, but I'm not sure.

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


int binsearch(char* word,int size,char* dictionary);

int main(void)
{
FILE* dict;
dict = fopen("dictionary.in","r");
int words;
fscanf(dict,"%d",&words);
int i;
char** dictionary[words];

for(i=0;(i<words);i++)
{
    dictionary[i] = malloc( 30 * sizeof(char*));
    fscanf(dict,"%s",&dictionary[i]);
    printf("%s\n",&dictionary[i]);
    printf("%s\n",&dictionary[i-1]);
}

printf("%s",&dictionary[5000]);

free(*dictionary);
free(dictionary);
fclose(dict);
return 0;
}

[WaltP]

What value do you expect

sizeof(char*)

to be?

[TreeBranches]

Hmm....is that allocating the size of a pointer to a char instead of the size of a char? I thought I was safe by slapping down a big number to multiply it by. I've tried replacing it with just sizeof(char) but I'm getting the same issue. Am I overlooking something about allocating space for a string? I'm pretty sure none of the words that I need to read in are longer than 20 chars.

[VernonDozier]

>> Hmm....is that allocating the size of a pointer to a char instead of the size of a char

Well, sizeof doesn't allocate anything, but combined with malloc, yes you're allocating memory for 30 pointers, not thirty chars.

I'm confused by line 16...

char** dictionary[words];

I can't tell whether this is supposed to be an array of strings or something else. Seems like you have an extra asterisk in there. For an array of strings I think you'd have two asterisks and no brackets, or one asterisk and brackets, but not two asterisks AND brackets, which would be an array of pointers to strings. You also have a static allocation of an array whose size (i.e. the words VARIABLE) isn't known at compile-time?

Seems like it's supposed to be an array of strings, so I'm thinking it needs to be this...

char** dictionary = malloc(words * sizeof(char*));

Then for each individual word, you'd do this...

dictionary[i] = malloc(30 * sizeof(char));

>> usually after the first four letters

Since chances are that the size of a pointer is 4 and a possible cause is an incorrect level of derefencing, that fact might add weight to that theory.

[TreeBranches]

That's right, my declarations had a few issues, and I've replaced everything you mentioned, by my problem is persisting. I'll try explaining it, but I'm still rather confused. The dictionary contains 25011 words. If I try print out the string at &dictionary[25010], I get zygote, which is the proper value. However, if I try printing out the word that comes before, &dictionary[25009], I should be getting "zurich", but instead, I get "zurizygote". It looks as if latest word, zygote, is overwriting the previous. If I try printing a much earlier word, it's very, very long, showing it's been overwritten many times, I suppose.

It seems again like I'm not allocating enough space for each string as I read them in, but I've changed my malloc commands to the below. I also tried to read the strings into a temp char array, and then copy that into my main dictionary array, but with the same results.

char** dictionary = malloc(words * sizeof(char*));
for(i=0;i<words;i++)
{
    dictionary[i] = malloc(30 * sizeof(char) );

        fscanf(dict,"%s",temp);

    strcpy(&dictionary[i],temp);

        for(j=0;j<=30;j++)
        {
            temp[j]='\0';
        }
    printf("%s\n",&dictionary[i]);
}

[VernonDozier]

Not sure where temp is declared. It serves no purpose as far as I can tell. You read into temp, then copy to your dictionary entry. Why not just read in directly?

Line 8 looks off.

strcpy(&dictionary[i],temp);

dictionary is a char** and therefore so is &dictionary (the & and the [] offset each other in terms of levels of pointers. Copying a string to a char** doesn't make sense. You want to copy it to a char*. dictionary is already a char*, so get rid of the leading &.

strcpy(dictionary[i],temp);

As far as line 4, I'm not sure it makes a difference, but a typecast would be advisable for readability and certainly couldn't hurt. If nothing else, it reminds you, the code writer, of what the types are.

dictionary[i] = (char*) malloc(30 * sizeof(char) );

Ditto line 1. Typecast to char**.

[VernonDozier]

Also see line 14. Same problem as line 8. &dictionary is a char**. You want a char*, so delete that leading &.

[TreeBranches]

Ah, I understand it now. That's exactly what was wrong. Thanks a lot!