Thread: Help with Malloc() function

Where does n get a value? Could you post a snippet of the input file? Doesn't the while loop consume the entire file before you get to malloc?

space = malloc(n*sizeof(char));

and it is not currently in used

You are wrong - it is in use, but it is not initialized

sizeof(char) is always 1, so you can leave it out of the expression if you like.

Looking at your code, I'm guessing that you want to read the file through first to see how many words are in it, and then allocate enough memory to hold each word, and then read each word from the file again. (If you want to get each line instead of each word, use fgets().) Is that right?

As I have outlined above, you'd need to use the rewind() function to reset the FILE pointer to the beginning of the file between the loops.

But there's a better way to do it. The function realloc() allows you to resize a memory block. So you can allocate as you go. Read a line from the file, allocate space for it, repeat until the get to the end of the file.

No matter which method you use, you need to allocate memory properly. You need an array of pointers, with enough elements in the array to point to each string, which also should be dynamically allocated. Or you could do this, I suppose:

Code:

char *lines[STRSIZE];

... = realloc(lines, sizeof(*lines) * n);
strcpy(lines[n-1], line);

Search around the board, and I'm sure you'll find several examples of dynamically allocating arrays of strings.

Could you post your current code?

As the previous bit of code you posted is severly broken, you have hopefully fixed the bugs pointed out above, so you probably should be closer to a solution now. If you haven't fixed the bugs pointed out above, please re-read the thread and make corrections, THEN post the code.

--
Mats

Originally Posted by xp5

Yes, the code is below:

Code:

...
  FILE *pf;
  char *space;
...
  pf = fopen("story.txt","r");
...
  space = (char*) malloc(pf);
...
}

What do you expect this code to do, because as far as I'm concerned, this shouldn't even compile. pf is a pointer to a file, and whilst this CAN be converted to an unsigned integer of type size_t with a case, it's almost certainly not the right argument for malloc - with or without cast. A memory address for a FILE object is nearly a random number, so you are asking for some random amount of memory. For example, the numeric value of the first file opened in my Microsoft Visual Studio environment has the value 0x49f420 - that means about 4.5MB. Which is a large amount of memory to ask for. If you have a different environment, you may find that the numeric value of pf is out of range for malloc alltogether.

Second, you should never cast the result of malloc().

--
Mats

You had the right idea in your original code, passing the number of words to malloc(). Except you used the uninitialized variable n instead of the variable pos, which you incremented to count the number of words in the file.

But stop for a moment and think about what you're allocating. If you were to code this program with a limit on the number of words in a file, say WORD_COUNT, as well as a limit on the length of each word, say WORD_LENGTH, then your declaration might look something like this:

Code:

char word[WORD_COUNT][WORD_LENGTH];

You'd read words into that array as follows:

Code:

  while(fscanf(pf,"%s",str) == 1)
  {
    strcpy(word[pos++], str);
  }

Now, what if you were to dynamically allocate memory for word[][] instead of using fixed limits? First let's get rid of the WORD_COUNT limit, because it seems more restrictive. You end up with a declaration like this:

Code:

char *(word[WORD_LENGTH]);

Now, assuming that you know the number of words in the file, you can allocate memory for that array with

Code:

word = malloc(sizeof(*word) * words);

Then you'd read the file in the same way as you did for the word[WORD_COUNT][WORD_LENGTH] array.

Side note: malloc()
The common idiom for malloc() is

Code:

p = malloc(sizeof(*p));

or

Code:

p = malloc(sizeof *p);

because it works no matter what type p is (as long as p is a non-void pointer).

The parentheses for sizeof() can be left out if you're taking the sizeof() a variable rather than a type such as int.

In this case, sizeof(*word) is equivalent to sizeof(char []) or sizeof(char *).

You could also eliminate WORD_LENGTH instead:

Code:

char *word[WORD_COUNT];

or

Code:

char *(word[WORD_COUNT]);

You wouldn't need to allocate memory before you read from the file, but your file reading loop would have to look something like

Code:

  while(fscanf(pf,"%s",str) == 1)
  {
    word[pos] = malloc(strlen(str) + 1);  /* +1 for NULL */
    strcpy(word[pos], str);
    pos ++;
  }

And then you'd need to free() the memory once you were done with it, of course.

But can we eliminate both restrictions on the dimensions? Will a declaration like this work?

Code:

char **word;

Yes, of course it will. But you'll need to call malloc() in two places, to allocate memory for the array of pointers, which will contain the same number of elements as the file contains words; and you'll also need to allocate memory for each word.

I won't post the code for this final version; you ought to be able to figure it out yourself from my previous examples. If you can't, just say so.

Finally, what about realloc()? All of the above assumes that you know how many words are in the file, which you do not. I don't feel like posting a realloc() example, but you can search around for one. Here's a really old thread of mine that demonstrates this sort of thing: http://cboard.cprogramming.com/showt...hlight=realloc

Oops, this

You could also eliminate WORD_LENGTH instead:

Code:

char *word[WORD_COUNT];

or

Code:

char *(word[WORD_COUNT]);

should have been

You could also eliminate WORD_LENGTH instead:

Code:

char *word[WORD_COUNT];

or

Code:

char (*word)[WORD_COUNT];

I'm glad it helped you understand dynamic memory allocation. If you have any other questions, just ask.