Explications for malloc ang some questions about pointers

R

Rano

/*
Hello,

I've got some troubles with a stupid program...
In fact, I just start with the C language and sometime
I don't understand how I really have to use malloc.
I've readden the FAQ http://www.eskimo.com/~scs/C-faq/faq.html
but it doesn't seem to answer my questions...

So, I've made an example behind, with some included questions...
If you can tell me what's wrong :)

(In fact, i'me trying to write a chat server to see how C works.
And somewherre in my code something like my example behind. (It's
to know whose (BB are users) are connected in a room (AA are rooms)
and in which rooms (AA) is connected a user (BB). But when I test,
with about 6 users, I've got a "pointer" bug. My room "pointer name"
point to the user name... :| And I don't know why, because "for me"
everything is right ^^)


Sorry for my bad English :)

*/


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

// ---------------- STRUCT ----------------

struct AAtmp
{
int name_length;
char * name;
int n;
struct BBtmp **bb;
};

struct BBtmp
{
int name_length;
char * name;
int n;
struct AAtmp **aa;
};

typedef struct AAtmp AA;
typedef struct BBtmp BB;


// ------------- FUNCTIONS --------------


AA *createAA(char *name, unsigned int name_length)
{
AA *a_tmp = malloc(sizeof(AA));
/*************************************************
Q1) Do you I have to malloc sizeof(AA) or sizeof(AA*) ? And Why ?
Because what I want is a AA*, but when I try to malloc sizeof(AA*)
the execution crash. (Freeze)
**************************************************/
memset(a_tmp, 0, sizeof(AA));

// The name
a_tmp->name_length = (name_length+1);
a_tmp->name = malloc(name_length+1);
memcpy(a_tmp->name, name, name_length+1);

// The BBs'array
a_tmp->n = 0;
a_tmp->bb = malloc(sizeof(BB **));
/************************************************
Q2) Do i have to malloc (sizeof(BB **) or sizeof(BB *)) ?
Both have a size of 4, but what is the most clean ?
If it's like the Q1), I suppose it's sizeof(BB *) no ?
************************************************/

return a_tmp;
}


BB *createBB(char *name, unsigned int name_length)
{
/*
same as AA but whith BB that comes AA
*/
BB *b_tmp = malloc(sizeof(BB));
memset(b_tmp, 0, sizeof(BB));
b_tmp->name_length = (name_length+1);
b_tmp->name = malloc(name_length+1);
memcpy(b_tmp->name, name, name_length+1);
b_tmp->n = 0;
b_tmp->aa = malloc(sizeof(AA **));

return b_tmp;
}




// ---------------- MAIN ----------------

int main()
{
int nbb, naa;
BB **BB_array;
AA **AA_array;
char *name;

nbb = 0;
naa = 0;
name = malloc(50);
BB_array = malloc(sizeof(BB **));
AA_array = malloc(sizeof(AA **));
/************************************************
Q3) Do i have to malloc these arrays ?
************************************************/

name = "Hello";
AA_array[naa++] = createAA(name, strlen(name));

name = "World";
BB_array[nbb++] = createBB(name, strlen(name));


AA_array[0]->bb[AA_array[0]->n] = BB_array[0];
/************************************************
Q4) Can I do that ? Or do I have to malloc (before) again and again
with something like :
AA_array[0]->bb[AA_array[0]->n] = malloc(sizeof(B *));
************************************************/


BB_array[0]->aa[BB_array[0]->n] = AA_array[0];
/************************************************
Q5) Can it be a problem to have what I do ? I mean, a pointer to a BB
in a AA that have got
itself a pointer to the BB (... that have got a pointer to the AA...)
************************************************/

return 0;
}


/*
Thanks :)
*/
 
N

Nick Patavalis

AA *createAA(char *name, unsigned int name_length)
{
AA *a_tmp = malloc(sizeof(AA));

You should malloc for sizeof(AA), or for sizeof(*a_tmp). And better
still, don't put this in the initializer:

AA *a_tmp;

a_tmp = malloc(sizeof(*a_tmp));

or

AA *a_tmp;

a_tmp = malloc(sizeof(AA));

/npat
 
J

Jens.Toerring

Rano said:
/*
Hello,
I've got some troubles with a stupid program...
In fact, I just start with the C language and sometime
I don't understand how I really have to use malloc.
I've readden the FAQ http://www.eskimo.com/~scs/C-faq/faq.html
but it doesn't seem to answer my questions...
So, I've made an example behind, with some included questions...
If you can tell me what's wrong :)
(In fact, i'me trying to write a chat server to see how C works.
And somewherre in my code something like my example behind. (It's
to know whose (BB are users) are connected in a room (AA are rooms)
and in which rooms (AA) is connected a user (BB). But when I test,
with about 6 users, I've got a "pointer" bug. My room "pointer name"
point to the user name... :| And I don't know why, because "for me"
everything is right ^^)
Sorry for my bad English :)

#include <stdlib.h>
#include <string.h>
// ---------------- STRUCT ----------------
struct AAtmp
{
int name_length;
char * name;
int n;
struct BBtmp **bb;
};
struct BBtmp
{
int name_length;
char * name;
int n;
struct AAtmp **aa;
};
typedef struct AAtmp AA;
typedef struct BBtmp BB;

// ------------- FUNCTIONS --------------

AA *createAA(char *name, unsigned int name_length)
{
AA *a_tmp = malloc(sizeof(AA));
/*************************************************
Q1) Do you I have to malloc sizeof(AA) or sizeof(AA*) ? And Why ?
Because what I want is a AA*, but when I try to malloc sizeof(AA*)
the execution crash. (Freeze)
**************************************************/

It looks as if you want memory for a single structure of type 'AAtmp'
(which you have typedef'ed to 'AA'). In that case the way you have
written it is absolutely correct - you don't want a pointer to it
(that you already got with your variable 'a_tmp') but memory for
such a structure itself, so you need the size of the structure and
not the size of a pointer to the structure.
memset(a_tmp, 0, sizeof(AA));

That seems to be rather useless (and could also be achieved in
a single step by using calloc() instead of malloc()).
// The name
a_tmp->name_length = (name_length+1);
a_tmp->name = malloc(name_length+1);
memcpy(a_tmp->name, name, name_length+1);
// The BBs'array
a_tmp->n = 0;
a_tmp->bb = malloc(sizeof(BB **));
/************************************************
Q2) Do i have to malloc (sizeof(BB **) or sizeof(BB *)) ?
Both have a size of 4, but what is the most clean ?
If it's like the Q1), I suppose it's sizeof(BB *) no ?
************************************************/

No, you need sizeof(BB *) here - You already have a pointer
to pointer to BB (the 'bb' element of the structure) and now
you need memory it's going to point to, and that's a pointer
to BB, so that's also what you must use as the "argument" to
sizeof (I am writing argument in quotes since sizeof isn't a
function but an operator, so it's not really an argument in
the sense of a function argument).

You can make life more simple for yourself by using the following
trick. Instead of using the type as the "argument" of sizeof you
can also use the variable that's on the left hand side of the
assignment, just adorned with one more '*' than you have on the
left hand side:

a_tmp->bb = malloc( sizeof *a_tmp->bb );

This also has the advantage that should you ever change the type
of 'a_tmp->bb' the above statement will still be correct without
any changes while, if you had used 'sizeof(BB *)', you would have
to find and change the line to reflect the changes you have made.
(You may have noticed that I here dropped the parenthesis around
the "argument" of sizeof - they are only required when it's a
type, bt not when it's a variable.)

One thing you should get used to is checking the return value
of malloc(). If you run out of memory malloc() returns NULL,
i.e. an invalid pointer. If you try to use that sometime later
the program is going to crash and you will have hard time to
find out why, while here you still have a good chance to
check such cases.
return a_tmp;
}
BB *createBB(char *name, unsigned int name_length)
{
/*
same as AA but whith BB that comes AA
*/
BB *b_tmp = malloc(sizeof(BB));
memset(b_tmp, 0, sizeof(BB));
b_tmp->name_length = (name_length+1);
b_tmp->name = malloc(name_length+1);
memcpy(b_tmp->name, name, name_length+1);

The use of strncpy() would look less obfuscated here. And since you
seem to be passing only strings to the function a simple strcpy()
would also do.
b_tmp->n = 0;
b_tmp->aa = malloc(sizeof(AA **));

Here you also need

b_tmp->aa = malloc(sizeof(AA *));

or

b_tmp->aa = malloc(sizeof *b_tmp->aa );
return b_tmp;
}
// ---------------- MAIN ----------------
int main()
{
int nbb, naa;
BB **BB_array;
AA **AA_array;
char *name;
nbb = 0;
naa = 0;
name = malloc(50);
BB_array = malloc(sizeof(BB **));
AA_array = malloc(sizeof(AA **));

Again you have one too many '*' here, you need

BB_array = malloc(sizeof(BB *));
AA_array = malloc(sizeof(AA *));

or

BB_array = malloc(sizeof *BB_array);
AA_array = malloc(sizeof *AA_array);
/************************************************
Q3) Do i have to malloc these arrays ?
************************************************/
name = "Hello";
AA_array[naa++] = createAA(name, strlen(name));
name = "World";
BB_array[nbb++] = createBB(name, strlen(name));

That's all fine - just remember that your AA_array and
BB_array only have a single element (more you didn't
allocate).

But you could get rid of the 'name' variable here, a simple

AA_array[naa++] = createAA("Hello", strlen("Hello"));

would also do nicely.
AA_array[0]->bb[AA_array[0]->n] = BB_array[0];
/************************************************
Q4) Can I do that ? Or do I have to malloc (before) again and again
with something like :
AA_array[0]->bb[AA_array[0]->n] = malloc(sizeof(B *));
************************************************/

Yes, you can. You make the pointer somewhere hidden in
what AA_array[0] is pointing to point to the first element
of the BB_Array. If you would assign the return value of
malloc() the pointer would point to somewhere else.

But it's getting awfully complicated to understand. While that,
of course, isn't a problem for the compiler other people reading
your code will probably have a hard time understanding what (and
why) you do all that (and in a few weeks time you also may have
this problem).
BB_array[0]->aa[BB_array[0]->n] = AA_array[0];
/************************************************
Q5) Can it be a problem to have what I do ? I mean, a pointer to a BB
in a AA that have got
itself a pointer to the BB (... that have got a pointer to the AA...)
************************************************/

No immediate problem - you just have just created some circular
relationship between the structures. If you don't start chasing
pointers until you come to some "final" element nothing bad is
going to happen. The only problem is that I really have no idea
what all this is supposed to be good for;-)
return 0;
}

What I don't understand is why you have to use double pointers all
of the time. As far as I can see you wouldn't have to use one in
the whole program - at least as far as it is finished yet or you
have posted it (there seem to be more since you were writing about
"6 users" qwhch wouldn't be possible with what you have shown).

I guess that you could just have

typedef struct AAtmp AA;
typedef struct BBtmp BB;

struct AAtmp {
size_t name_length;
char * name;
int n;
BB *bb;
};

struct BBtmp {
size_t name_length;
char * name;
int n;
AA *aa;
};

And in main() you now would have

AA *AA_array;
BB *BB_array;

AA_array = malloc( sizeof *AA_array );
BB_array = malloc( sizeof *BB_array );

And then initialize the first (and only) element of AA_array by

createAA( AA_array, name );
createBB( BB_array, name );

with

void createAA( AA *a_tmp, char *name )
{
a_tmp->name_length = strlen( name ) + 1;
a_tmp->name = malloc( a_tmp->name_length );
strcpy( a_tmp->name, name );
a_tmp->n = 0;
a_tmp->bb = NULL; /* mark it as still unused */
}

Of course you would also need the same changes for createBB().

Later in main() you would do e.g. (there are several ways to write
that):

*( AA_array[ 0 ].bb + AA_array[ 0 ].n ] ) = &BB_array[ 0 ];
*( BB_array[ 0 ].aa + BB_array[ 0 ].n ] ) = &AA_array[ 0 ];

I guess you would get exactly the same effect but with a bit
less of memory consumption and a structure of the program
that's quite a bit easier to understand. On the other hand
you might be up to something different that I don't understand
and where your use of double pointers could be required.

Regards, Jens
 
A

Alex Fraser

Nick Patavalis said:
You should malloc for sizeof(AA), or for sizeof(*a_tmp).

The brackets are only necessary when sizeof is applied to a type. To me, the
most readable form is:

AA *a_tmp = malloc(sizeof *a_tmp);
And better still, don't put this in the initializer:

Why not?

Alex
 
N

Nick Patavalis

AA *a_tmp = malloc(sizeof *a_tmp);


Why not?

Basically I looks ugly. Furthermore, if you put several such things in
the initializers, the you will have to remember to check if the
mallocs were successful, inside the function body, in a place
different from where the mallocs were called, and this is a bit
distracting.

/npat
 
M

Michael Mair

Cheerio,


Assumption (*): You want to store several pointers to BB in bb
and several pointers to AA in aa and n is the maximum index
you can use with aa and bb, respectively; that is, it is the next
index to be written to, too.
HINT: give us comments so that we know what you mean...
I will refer to (*) later on.


As Jens pointed out: sizeof(BB *)
If (*) is valid, the second line should read:
a_tmp->bb = malloc(((a_tmp->n)+1)*sizeof(BB **));
A second thing: _ALWAYS_ check the return value of malloc!
if (a_tmp->bb==NULL) {
/* Error message and exit */
}

.... ....
Just the same.
(**) Okay, here I think you are trying to do the same again as in (*).
Use
BB_array = malloc((nbb+1)*sizeof(BB *));
AA_array = malloc((naa+1)*sizeof(AA *));
name = "Hello";
AA_array[naa++] = createAA(name, strlen(name));

name = "World";
BB_array[nbb++] = createBB(name, strlen(name));


That's all fine - just remember that your AA_array and
BB_array only have a single element (more you didn't
allocate).

@Jens: I think that's the crux.
@OP: And -- under the assumptions (*) and (**) -- your mistake becomes
clear:
You are confusing pointers and pointers to pointers.
You can store what createAA() returns exactly one time.
naa is the _maximal_ index. As long as you do not get more memory
for AA_array to point to, you can store only one pointer "in"
AA_array. I would do it differently, for example
size_t n_b, used_b;

n_b = 1; /* initial number of elements to be stored in BB_Array */
used_b = 0; /* How many are already used? */

....

BB_array[used_b++] = createBB(name, strlen(name));
if (used_b == n_b) {
BB **tmp;
if ( (tmp=realloc(BB_array, 2*n_b*sizeof(BB *) == NULL ) {
/* Error message; maximum number of BB * reached */
/* Do whatever you want to */
}
else {
n_b *= 2;
BB_array = tmp;
}
}
Instead of inserting that always, you can use a function instead
which tests used_b and n_b and reallocates BB_array automatically.

Maybe an ASCII picture is somewhat more enlightening:
___________
BB_array -----> BB_Array[0] -----> a BB variable
___________
BB_Array[1] -----> a BB variable
___________
....
___________
BB_Array[used_b-1] -----> last BB variable created
___________
BB_Array[used_b] here, the next pointer to a BB variable
will be stored
___________
....
___________
BB_Array[n_b-1] last possible position within the
allocated memory for BB_array.
___________

So, you should get a clear picture of what a pointer to a pointer does
and why you need memory to work with it...
For AA and BB you need a similar construction with "max" and "used".
BTW: It is usually safer to use size_t for indices of arrays instead of
int. In this case, int probably will not hurt but as soon as you use
comparatively small data types (a few bytes only) and huge indices,
you might run out of the int range whereas size_t is guaranteed to be
big enough to contain the number of bytes in your memory.

....
What I don't understand is why you have to use double pointers all
of the time. As far as I can see you wouldn't have to use one in
the whole program - at least as far as it is finished yet or you
have posted it (there seem to be more since you were writing about
"6 users" qwhch wouldn't be possible with what you have shown).

@Jens: I hope my guess is right, then this is a little bit more clear...


AA *AA_array;
BB *BB_array;

AA_array = malloc( sizeof *AA_array );
BB_array = malloc( sizeof *BB_array );

@Jens: *g* Please give us the check, too...
If the OP sees it often enough he might remember :)


HTH
Michael
 
R

Rano

Many many thanks :)

I've realised what I didn't understood...
the size we have to give to malloc is the size of the object the
pointer points TO ! And not the size of the pointer :) Now it's so
"evident" that I feel a bit stupid ^^

Really, many thanks for these explications :)

Now I've found why my "chat server" (that I told in my first post)
didn't work, with more thant exactly 4 users. It was caused by memory
allocation...

My array :
AA **aa_aray;

that I initialised with malloc(sizeof(A *)) was good to get ONE
pointer like in my example but in my real program I've got many
pointers, so I needed to realloc the size of this array to "stock" the
other pointers :) (... "what it points to !")

Once again, many thanks :)
 
O

Old Wolf

I've got some troubles with a stupid program...
In fact, I just start with the C language and sometime
I don't understand how I really have to use malloc.

A pointer is a thing that contains the address of some
block of memory somewhere.

"malloc" allocates a number of bytes and returns a pointer to
the first of the bytes it allocated.

If you want to create a new object of type T, then:
T *ptr = malloc( sizeof(T) );
because the number of bytes you need to store a T object is
sizeof(T) , not sizeof(T*). Now 'ptr' points to (ie. is the
address of) the object you created.

Also, every time you call "malloc" you should check that the
result is not NULL (ie. indicating that you ran out of memory),
and do something graceful (eg. print "out of memory" and exit()).

You should also have one "free" corresponding to each "malloc",
to release the memory you allocated.
struct AAtmp
{
int name_length;
char * name;
int n;
struct BBtmp **bb;
};
typedef struct AAtmp AA;

It's safe to use the same name for the 'struct tag' as for the typedef,
ie. use AA instead of AAtmp.
AA *createAA(char *name, unsigned int name_length)

It is easier to use C strings instead of length-counted strings.
C strings use the character 0 to indicate their end. So:

AA *createAA(char const *name)

(the 'const' indicates that what 'name' points to won't be modified.
It helps to avoid you accidentally modifying it).
{
AA *a_tmp = malloc(sizeof(AA));
memset(a_tmp, 0, sizeof(AA));

You have created an object the right size to be an "AA".
"a_tmp" points to it.
// The name
a_tmp->name_length = (name_length+1);
a_tmp->name = malloc(name_length+1);
memcpy(a_tmp->name, name, name_length+1);

I would suggest that "name_length" is not a good name for a
variable if it is actually one more than "name_length".
In fact, using C strings:

a_tmp->name = malloc( strlen(name) + 1 );
strcpy(a_tmp->name, name);
// The BBs'array
a_tmp->n = 0;
a_tmp->bb = malloc(sizeof(BB **));
/************************************************
Q2) Do i have to malloc (sizeof(BB **) or sizeof(BB *)) ?
Both have a size of 4, but what is the most clean ?
If it's like the Q1), I suppose it's sizeof(BB *) no ?
************************************************/

I am guessing (see my further comments later..) that this is
meant to be a list of pointers-to-BB, where the things that
are pointed to, are in main()'s "b_tmp". But there are no items
in your list yet, so you would be best to set a_tmp->bb = NULL.
return a_tmp;
}
// ---------------- MAIN ----------------

int main()
{
int nbb, naa;
BB **BB_array;
AA **AA_array;
nbb = 0;
naa = 0;
BB_array = malloc(sizeof(BB **));
AA_array = malloc(sizeof(AA **));

AA_array is a pointer to (an array of) POINTERS to AA.
At any point in time, you will have some number of pointers
in this array.

It looks as if you want to start off having one pointer-to-AA
in the array, so:

AA_array = malloc( sizeof(AA *) );
naa = 1;

Now there is a memory region capable of holding one pointer-to-AA,
and AA_array points to that region.

If, in future, you want to add another person at the end of the list:
naa++;
AA_array = realloc( AA_array, naa * sizeof(AA *) );
AA_array[naa - 1] = createAA("new person");

Or to delete the last person:
free( AA_array[--naa] );
AA_array = realloc( AA_array, naa * sizeof(AA *) );

The 'realloc' function obtains a new block of memory in the size
you specify, and copies over the contents from the old block
(ie. you have changed the size of your array).
char *name;
name = malloc(50);

You allocate 50 bytes and point 'name' to it...
/************************************************
Q3) Do i have to malloc these arrays ?
************************************************/

name = "Hello";

You point 'name' to "Hello", which is already sitting in memory
at a quite different place to the 50 bytes you just allocated. You
have now leaked the original 50 bytes, which have nothing pointing
to them any more.
AA_array[naa++] = createAA(name, strlen(name));

It would have been be good design to keep 'naa's updating as close
as possible to the point where you allocate AA_array.

Simpler would have been:
AA_array[0] = createAA("Hello");
AA_array[0]->bb[AA_array[0]->n] = BB_array[0];
/************************************************
Q4) Can I do that ? Or do I have to malloc (before) again and again
with something like :
AA_array[0]->bb[AA_array[0]->n] = malloc(sizeof(B *));
************************************************/

I am guessing that you would want to make the chat-rooms(BB) maintain
a list of who(AA) is in it, and also have each person(AA) maintain
a list of which chat-rooms(BB) they are in. You could do this, but
it would involve quite a lot of work every time someone entered or
left a room.

If it were me designing this project, I would maintain a list
of pairs (AA *, BB *), indicating that person A is in room B,
rather than trying to store this information inside the
AA and BB objects.
You have to go through the lists a_tmp and b_tmp, freeing each
member of them (and for each member, free its name; you probably
should have a function deleteAA() and deleteBB() to do this),
and then finally free a_tmp and b_tmp themselves. Although
you would get away with not freeing them (the OS will free all
memory your application allocated, when it exits).
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,483
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top