Memory allocation problem

  • Follow


I am writing code for a dictionary type in c, but i have some problems
allocating the memory for it. The dictionary type itself is a struct
with the following contents:

typedef struct {
	char ** keys;
	char ** content;
	int count; /* Amount of space actually used */
	int allocated_space; /* Amount of space allocated for the keys and
content */
} dict_t;

The function that allocates the memory goes like this:

dict_t * dictWithAllocatedSpace(dict_t * dictionary, int size)
{
	dictionary = (dict_t *)malloc(sizeof(dict_t));
	dictionary->allocated_space = size;
	dictionary->keys = (char **)malloc(size);
	dictionary->content = (char **)malloc(size);
	dictionary->count = 0;
	return dictionary;
}

when this function is executed the pointer it returns is not NULL, but
when i try to access one of its members, i get a bus error. I probably
made a terrible mistake, but i have no idea what that could be.
Any help is appreciated!

Ruben

0
Reply ruben.de.visscher (9) 2/4/2006 9:59:38 AM

RubenDV wrote:
> I am writing code for a dictionary type in c, but i have some problems
> allocating the memory for it. The dictionary type itself is a struct
> with the following contents:
> 
> typedef struct {
> 	char ** keys;
> 	char ** content;
> 	int count; /* Amount of space actually used */
> 	int allocated_space; /* Amount of space allocated for the keys and
> content */
> } dict_t;
> 
> The function that allocates the memory goes like this:
> 
> dict_t * dictWithAllocatedSpace(dict_t * dictionary, int size)
> {
> 	dictionary = (dict_t *)malloc(sizeof(dict_t));
> 	dictionary->allocated_space = size;
> 	dictionary->keys = (char **)malloc(size);
> 	dictionary->content = (char **)malloc(size);
> 	dictionary->count = 0;
> 	return dictionary;
> }
> 

Let's rephrase it.

dict_t* dictWithAllocatedSpace(int size)
{
	dict_t *dictionary = malloc(sizeof *dictionary); /* do not cast malloc */
	if (dictionary==NULL) /* always check for NULL */
	{
		return NULL;
	}
	dictionary->allocated_space = size;
	dictionary->keys = malloc(size*sizeof *(dictionary->keys));
         if (dictionary->keys==NULL)
	{
		free(dictionary);
		return NULL;
	}
	dictionary->content = malloc(size*sizeof *(dictionary->content));
	if (dictionary->content==NULL)
	{
		free(dictionary->keys);
		free(dictionary);
		return NULL;
	}
	dictionary->count = 0;
	return dictionary;
}

> when this function is executed the pointer it returns is not NULL, but
> when i try to access one of its members, i get a bus error. I probably
> made a terrible mistake, but i have no idea what that could be.
> Any help is appreciated!
> 

After the changes above, you are sure that if you get a NULL, something 
has gone wrong. Check if NULL is returned...

If this does not solve anything, then post more code...

> Ruben
> 
0
Reply ipapadop (140) 2/4/2006 10:20:35 AM


Yes it works now. But could you explain some more where i was wrong? I
would not want to make the same mistakes again. And thanks a lot for
helping me out!

0
Reply ruben.de.visscher (9) 2/4/2006 11:17:41 AM

RubenDV wrote:
> Yes it works now. But could you explain some more where i was wrong? I
> would not want to make the same mistakes again. And thanks a lot for
> helping me out!

Please quote sufficient context -- otherwise people may
not know what you talk about. In this case, it becomes
unreasonably hard to point you to your error in the code.

For once, I will quote the relevant parts of your original
message.

> typedef struct {
> 	char ** keys;
> 	char ** content;
> 	int count; /* Amount of space actually used */
> 	int allocated_space; /* Amount of space allocated for the keys and
> content */
> } dict_t;
> 
> The function that allocates the memory goes like this:
> 
> dict_t * dictWithAllocatedSpace(dict_t * dictionary, int size)
 > {

As you did not show us a compiling minimal example, we
are reduced to guesses. In this case, though, there is
a guess:
You have a completely useless parameter: dictionary.
If you tried something along the lines

  dict_t *mydict;
  if (NULL == dictWithAllocatedSpace(mydict, DICT_SIZE))
  {
    .... /* error handling */
  }

then you passed an uninitialised value (the arbitrary value
mydict has without initialisation); this "value" became the
value of dictionary; you then overwrote dictionary's value.
However, the original variable, mydict, was not modified by
what happened to a copy of its value.
So, in order to modify mydict, you have to pass its address.
Say, we have

dict_t * dictWithAllocatedSpace(dict_t ** pdict, int size)
{
  dict_t *dictionary;
  *pdict = NULL;

> 	dictionary = (dict_t *)malloc(sizeof(dict_t));

Bad: Casting the return value of malloc().
Use malloc() like this:
  dictionary = malloc(sizeof *dictionary);
Worse: No checking of the return value against NULL.

> 	dictionary->allocated_space = size;
> 	dictionary->keys = (char **)malloc(size);
> 	dictionary->content = (char **)malloc(size);

Now, apart from
  dictionary->keys = malloc(size * sizeof *dictionary->keys);
the not checking for return values may really bite you
here:
You return a non-null pointer if the first allocation
was successful but the second and third allocations may
have gone wrong.

> 	dictionary->count = 0;

  *pdict = dictionary;

> 	return dictionary;
> }

Cheers
  Michael
-- 
E-Mail: Mine is an   /at/ gmx /dot/ de   address.
0
Reply Michael.Mair (1492) 2/4/2006 11:36:06 AM

3 Replies
30 Views

(page loaded in 0.264 seconds)


Reply: