Dynamic array of structures with a dynamic arrays inside of it

Hi everyone!
So what is I'm supposed to do, is kind of a index program, which works, for example, like that:
./a.out FILE word1 word2 word3...
it should show:
word1 accused in lines 1 2 3
word 2 acussed in lines 4, 5, 6
etc..

but I've got some problem with structures that keeps informations about words;

heres the part of program code and a test function for it

Code:

---

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

typedef struct {
char *word; // string contains a word
int how_many; // how many times word appears in file
int *numery; // in which lines word appers
} storage;

storage* init(  int argc, char **argv)
{
    storage **words; // thats is our dynamic array of structures //
    int i;
    for( i = 0; i < argc - 1; i++ )
    {
        words[i] = malloc( sizeof(storage) );
    }
    for( i = 0; i < argc - 1 ; i++)
    {
        words[i]->word = malloc( strlen( argv[i+1] ) * sizeof words[i]->word );
    words[i] = realloc(words[i], sizeof(storage));
        strcpy( words[i]->word, argv[i+1] );
    printf( "\n%s\n", words[i] -> word );
    }

    return *words;

}
int main(int argc, char **argv ){
    int i;
    storage **words;
    *words = init( argc, argv );   
    for( i = 0; i < argc - 1 ; i++ )
    {
        printf("\n%s\n", words[i] -> word );
    }

    return 0;
}

---

it looks like something is messed with malloc functions, and im locating memory unproperly..

please help, i dont know where to looking for bugs
thanks and sorry for my bad english

You've messed up more than your malloc() calls.

In init(), words is an uninitialised pointer. The first thing you then do is access words[i]. That gives undefined behaviour.

You need to make up your mind about whether you want words to of type storage * (only one asterix) or storage ** (which is what you have). Your usage of the words is inconsistent.

Thanks for the answer.
Honestly, I've got a big problem with when i need to use * and when **. My college teacher tought me that when I'm creating a function, which creates a table i need to use ** and then return *.

So can i simply declare in init()

Code:

---

storage *words;

---

and then

Code:

---

return word;

---

?

Will this work for my problem?

Quote:

---

Originally Posted by mdarda

Honestly, I've got a big problem with when i need to use * and when **. My college teacher tought me that when I'm creating a function, which creates a table i need to use ** and then return *.

---

That's rubbish. Sometimes you need to do that, but most times, it is unnecessary.

From information, you've provided, declaring words as storage * and returning words will work. You will need to change how you set up elements of the dynamically allocated array (using . rather than ->, etc).

Rule of thumb: use no more levels of indirection than necessary. "Indirection" means levels of pointer to pointer to ....

I've rewritten the program

Code:

---

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

typedef struct {
    char *word;
} storage;

storage* init(int argc, char **argv) {
    storage *words = NULL;

    int i;

    for(i = 0; i < argc - 2; i++) {
        words = malloc(sizeof(storage));
        words++;
    }

    for(i = 0; i < argc - 2; i++) {
        words[i].word = malloc(strlen(argv[i+1])+1);
        strcpy(words[i].word, argv[2+i]);
        words = realloc(words, sizeof(storage));
        words++;
    }

    return words;

}

int main(int argc, char **argv) {

    FILE *in = argc > 1 ? fopen(argv[1], "r") : stdin;
    int i;
    storage *words = NULL;
    words = init(argc, argv);

    for(i = 0; i < argc - 2; i++) {
        puts(words[i].word);
    }

    return 0;
}

---

Now, here is the output:

Code:

---

michal@MICHAL-UBUNTU:~$ ./a.out dane.txt slowo bocian gesc
slowo
bocian
gesc
michal@MICHAL-UBUNTU:~$ ./a.out dane.txt slowo bocian gesc gesciej


bocian
gesc
gesciej
michal@MICHAL-UBUNTU:~$ ./a.out dane.txt slowo bocian gesc gesciej szawsze
A%

bocian
gesc
gesciej
szawsze
michal@MICHAL-UBUNTU:~$ ./a.out dane.txt slowo bocian gesc gesciej szawsze slewac
0Q�

bocian
gesc
gesciej
szawsze
slewac
michal@MICHAL-UBUNTU:~$ ./a.out dane.txt slowo bocian gesc gesciej szawsze slewac ekdsngv
PA
bocian
gesc
gesciej
szawsze
slewac
ekdsngv
michal@MICHAL-UBUNTU:~$ ./a.out dane.txt slowo bocian gesc gesciej szawsze slewac ekdsngv nsjgdsjngs
p
�
gesc
gesciej
szawsze
slewac
ekdsngv
nsjgdsjngs

---

As you can see I'm getting some weird symbols, I know that means the program gets to out of memory he should be.. So now I've got some messed up malloc calls right? Can you please provide me to the right solution?

From your code sample in mesage 5 ... delete lines 16 and 23 recompile and test again

You are disturbing the root pointer to your array... a major no no when using malloc.

Quote:

---

Originally Posted by mdarda

Code:

---

    for(i = 0; i < argc - 2; i++) {
        words = malloc(sizeof(storage));
        words++;
    }

---

---

Would you care to explain what, exactly, you think this loop achieves?

In your code, words should never need to be incremented, and realloc() is not needed. Use pointer arithmetic or array indexing. Not both together.

Quote:

---

Originally Posted by grumpy

Would you care to explain what, exactly, you think this loop achieves?

In your code, words should never need to be incremented, and realloc() is not needed. Use pointer arithmetic or array indexing. Not both together.

---

I thought that with the for statement i can alloc space for argc-2 pointers (words pointers) thats why i'm incementing word++ that i could use them as a static array with [i] index, am I wrong?

Sheesh, are CT and grumpy jacking you around? ;)

Quote:

---

Originally Posted by mdarda

I thought that with the for statement i can alloc space for argc-2 pointers (words pointers) thats why i'm incementing word++ that i could use them as a static array with [i] index, am I wrong?

---

You do allocate the space, and that would be okay except:

1) words itself is supposed to be an array of pointers, but you never initialize it as such. Not only does every pointer in the array need to be allocated dynamically, but so does the array itself (see below).
2) You increment words in the initial for loop, so at the end, words does not point to the same place anymore. You would then need to rewind it:

Code:

---

words -= i; // pointer arithmetic uses unit size of type

---

However, that is sort of an unorthodox method. Your original for(i) loop in post #1 was correct, just you did not allocate for the array before you allocated its members.

Here's an example of part of what you are trying to do. It reads a list of words from standard input into an array of structs, dynamically allocated. You must specify how many words via a command line parameter, eg:

echo "one two three four five" | ./a.out 5

Code:

---

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

typedef struct {
        char *word;
        int count;
} storage;


storage *addWord (char *input) {
// allocate, initialize, and return a new struct
        storage *rv = malloc(sizeof(storage));
        rv->word = malloc(strlen(input));
        strcpy(rv->word, input);
        rv->count = 1;
        return rv;
}

void freeStorage (storage **p) {
// deallocate struct
        free((*p)->word);
        free(*p);
}

int main(int argc, char *argv[]) {
        int i, total = strtol(argv[1], NULL, 0);
// declare AND INITIALIZE words properly
        storage **words = malloc(total * sizeof(storage*));
        char data[1024];

// simultate file input
        for (i = 0; i < total; i++) {
        // check for error when reading
                if (fscanf(stdin, "%s", data) != 1) break; 
                words[i] = addWord(data);
        }

// check
        for (i = 0; i < total; i++)
                printf("%s (%d)\n", words[i]->word, words[i]->count);

// cleanup
        for (i = 0; i < total; i++) freeStorage(&words[i]);
        free(words);

        return 0;
}

---

So there is your **pointer array, and a function which returns a *pointer designed for it. Notice the malloc() on words is for sizeof(storage*) not sizeof(storage) -- the second one is used to allocate structs in the for loop, but the first one is for the pointers to the structs. Make sense?

Output:

one (1)
two (1)
three (1)
four (1)
five (1)

Quote:

---

Originally Posted by mdarda

I thought that with the for statement i can alloc space for argc-2 pointers (words pointers) thats why i'm incementing word++ that i could use them as a static array with [i] index, am I wrong?

---

Absolutely and utterly wrong.

If you want to allocate an array of n Foo's, change the value you supply to malloc(). For example, Foo *p = malloc(n * sizeof(Foo)). You don't call malloc(sizeof(Foo)) n times and you certainly don't increment the pointer.

Quote:

---

Originally Posted by MK27

Sheesh, are CT and grumpy jacking you around? ;)

---

No we are not. The OP has showed evidence of a deep misunderstanding of what he is doing. I was trying to understand the extent of that misunderstanding in order to correct it.

Incidentally, you have the lengths of strings wrong in your example.

Quote:

---

Originally Posted by grumpy

Incidentally, you have the lengths of strings wrong in your example.

---

Good catch, too bad I can't edit it. So line 14 from post #9:

Code:

---

    rv->word = malloc(strlen(input));
// should be
    rv->word = malloc(strlen(input)+1);

---