Declaring one more variable crashes program.

Hello guys.I am having this problem and i cant understand why.This my code

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(){
FILE *fp;
int c;char *str;
int str_n=0;
int maxstrl=-1;
int k;



fp = fopen("text.txt", "r");

do {
c=fscanf(fp, "%s",str);

if (c!=EOF){
 printf("%s",str);
 str_n++;


}
} while (c != EOF);
fclose(fp);

printf("\n%d",maxstrl);





return 0;
}

---

Once i remove int k; from the place i declare variables it works.But the problem occurs even when i remove maxstrl.My aim is to find the max string lenght and the number of words so i can allocate a two dimensional array with strings but i cant expain why the hell does this happen.Thanks in advance,i am really desperate.

> char *str;
An uninitialised pointer.

> c=fscanf(fp, "%s",str);
This writes characters all over some random piece of memory.
Depending on your luck, the result is anywhere between "it works" and "where's my OS gone!?".

Try instead with a
char str[100];

str is just a pointer - it doesn't have any allocated memory set aside to hold a string, yet.

Realistically, unless you're text contains the silly scientific descriptions they call "words" (and they aren't), then no word in English is longer than 30 letters. So making an array of words[10000][30] is no problem, as long as you don't create it on the local functions stack memory - which is quite limited.

Two solutions to get the memory from the heap:

1) Use malloc() - be sure to include stdlib.h in the program
or
2) make the array before main(), in global space

antidisestablishmentarianism - only 28 letters.

Quote:

---

Originally Posted by Salem

> char *str;
An uninitialised pointer.

> c=fscanf(fp, "%s",str);
This writes characters all over some random piece of memory.
Depending on your luck, the result is anywhere between "it works" and "where's my OS gone!?".

Try instead with a
char str[100];

---

The problem is that i dont know the number of words i am about to fscanf so i need to put them to a temporary place,and than later place them in a dynamic array(therefore i need to count the amount of words with str_n.Why doesnt the pointer overwrite the index so it can use the same adress as the value before?Thanks.

Quote:

---

Originally Posted by Adak

str is just a pointer - it doesn't have any allocated memory set aside to hold a string, yet.

Realistically, unless you're text contains the silly scientific descriptions they call "words" (and they aren't), then no word in English is longer than 30 letters. So making an array of words[10000][30] is no problem, as long as you don't create it on the local functions stack memory - which is quite limited.

Two solutions to get the memory from the heap:

1) Use malloc() - be sure to include stdlib.h in the program
or
2) make the array before main(), in global space

antidisestablishmentarianism - only 28 letters.

---

Well i am planing to find the max lenght that i read thought the loop using strlen() and a variable maxlenght.Therefore i will create an array[str_n][maxlenght].Sorry for calling it words.

> The problem is that i dont know the number of words i am about to fscanf
Well if you don't, then C won't magically fill in the gaps for you.

As Adak points out, an array of 30 should be enough for most English text, so begin with.

Code:

---

char word[30];
while ( fscanf(fp,"%29s",word) == 1 ) {
    numWords++;
}
printf("Found %d words in the file\n", numWords );

---

Having found the number of words, you can do this.

Code:

---

char (*allWords)[30] = malloc( numWords * sizeof(*allWords) );

---

This creates your dynamic array, which is large enough to store a number of words from the file. Each allWords[i] is an array of 30 chars to store a single word in.


Call rewind(fp) and repeat the fscanf calls.

Quote:

---

Originally Posted by Konstantinos

Why doesnt the pointer overwrite the index so it can use the same adress as the value before?

---

You're missing the point of previous explanations. A pointer does not overwrite anything. The pointer contains an address in memory. However, an uninitialised pointer does not point at a valid area in memory. In your code, fscanf(fp, "%s", str) assumes that str points at a valid area of memory but, since str is uninitialised, it does not point at a valid area of memory. The fscanf() call will therefore merrily overwrite memory it shouldn't.

Quote:

---

Originally Posted by Konstantinos

Well i am planing to find the max lenght that i read thought the loop using strlen() and a variable maxlenght.Therefore i will create an array[str_n][maxlenght].

---

Your plan is flawed. To do what you want, the fscanf() call needs to be supplied with memory it can write the string data to (i.e. str needs to point to a valid area of memory large enough to contain the data read by the fscanf() call. It is only after that that strlen() can be used, because strlen() ALSO assumes its argument is a valid area of memory containing a sequence of characters, with the last in the sequence being zero.

To do what you want, however, you need to compute the values of str_n and maxlenght (sic!) before the loop that reads the words.

One option is to initialise str_n and maxlenght with values larger than ever needed. One problem with that is that you might use more memory than needed (eg if str_n and maxlenght are significantly larger than needed). Another problem is that, if your file requires a larger value of str_n or maxlenght than you choose, then your code will again be in the situation of tromping memory it shouldn't (eg trying to read a 65 characters into a 50 character array).

An alternative is to read the file in two passes. The first pass computes the values needed for str_n and maxlenght, and allocates the arrays accordingly. The second pass makes use of the arrays, and will not tromp memory (since the arrays are large enough to hold the data from the file). A problem with the two-pass approach is that it is often slower (it reads the whole file twice rather than once, and I/O is typically a slow operation).


Whichever option you choose, however, you can't rely on magic happening because you are using pointers. It is your responsibility, when using a pointer (eg supplying a pointer to fscanf()) to ensure that pointer points at sufficient memory. A pointer does not magically come to point at valid memory, unless your code explicitly ensures that happens.

Quote:

---

Originally Posted by Salem

> The problem is that i dont know the number of words i am about to fscanf
Well if you don't, then C won't magically fill in the gaps for you.

As Adak points out, an array of 30 should be enough for most English text, so begin with.

Code:

---

char word[30];
while ( fscanf(fp,"%29s",word) == 1 ) {
    numWords++;
}
printf("Found %d words in the file\n", numWords );

---

Having found the number of words, you can do this.

Code:

---

char (*allWords)[30] = malloc( numWords * sizeof(*allWords) );

---

This creates your dynamic array, which is large enough to store a number of words from the file. Each allWords[i] is an array of 30 chars to store a single word in.


Call rewind(fp) and repeat the fscanf calls.

---

I dont get why sizeof(*allWords) and what is the meaning of it in this situation.Thanks everyone for helping me and for there valuable time.

sizeof(*allWords) is the same as sizeof(allWords[0]) which is the same as sizeof(char[30]) because that is the type of what allWords points to.

In general you can do the following to allocate n items of type T:

Code:

---

T *ptr;
ptr = malloc(n * sizeof(*ptr));

---

Using *ptr rather than T allows for you to change T later without having to look for other places where you might have used T.

My suggestion is you start with
char allWords[1000][30];
and a small text file, and get that working.

Then we can perhaps explain some baby-steps to how malloc works, because at the moment all the advise is just flying over your head.

What is the goal of the program? To save all the words into a two-dimensional array? Another way is to make an array of strings. For example let's allocate some memory called dict to store the total list of words, and then allocate an array of pointers called list, each member of which points to the appropriate spot in dict.

Code:

---

#define DICTMAX 1000000
#define LISTMAX 10000
 char buf[30];
 char dict[DICTMAX] = "";
 char *dictend = &dict[DICTMAX];
 char *list
[LISTMAX] = {}; 
 char **listend = &list
[LISTMAX];
 void listprint(char **list);
 
int main()
{
    FILE *fp = fopen("in.txt", "r");
    char *d = dict;
    char **l = list;
    for (; fscanf(fp, "%29s", buf)==1; d += strlen(buf)+1, l++)
    {
        strncpy(d, buf, dictend-d);
        *l = d;
        if (dictend-d < strlen(buf)+1)
            break;  // no more room in words
        if (listend-l == 0)
            break;  // no more room in list
    }
    fclose(fp);

    int numwords = l-list;
    printf("Number of words: %d\n", numwords);
    listprint(list);
    return 0;
}

 void listprint(char **list)
{
    for (char **l = list; listend-l != 0; l++)
        printf("%s\n", *l);
}

---

Notes:
1. words are defined as a sequence of non-whitespace characters
2. no word is longer than 29 characters; this number can be increased as needed without increasing the size of dict or list.
3. the total number of words is not greater than LISTMAX
4. the total number of word characters is not greater than DICTMAX+k, where k is the number of words encountered so far