Confused with malloc

[Richard Hunt]

I'm sorry for asking such a silly question, but I can't quite get my head
around malloc. Using gcc I have always programmed in a lax C/C++ hybrid
(which I suppose is actually c++). But I have started messing around in Plan
9, and that sort of thing is totally no go there .

Is this correct to allocate memory for my struct? It works on my computer,
but I'm suspicious that I'm doing it wrong.

--

struct NODE;

struct NODE {
char string[80];
struct NODE *next;
}

int main()
{
struct NODE *first;

first = (struct NODE) malloc(sizeof(struct NODE));

first->next = (struct NODE) malloc(sizeof(struct NODE));

free(first);

return 0;
}

Is it necessary to malloc the struct 'first'?

 

[Richard Hunt]

Sorry, I see one error in that code already, there should be a semicolon at
the end of the struct .

 

[Kevin Goodsell]

I'm sorry for asking such a silly question, but I can't quite get my head
around malloc. Using gcc I have always programmed in a lax C/C++ hybrid
(which I suppose is actually c++). But I have started messing around in Plan
9, and that sort of thing is totally no go there .

Is this correct to allocate memory for my struct? It works on my computer,
but I'm suspicious that I'm doing it wrong.

No, it's not correct. It shouldn't even compile. Here are the
diagnostics I got:

"C:\cygwin\bin\gcc.exe" -W -Wall -Wcast-align -Wwrite-strings
-Wno-sign-compare -Wno-unused-parameter -Wpointer-arith -ansi
-pedantic-errors -ggdb -c fun.c
fun.c:9: error: two or more data types in declaration of `main'
fun.c: In function `main':
fun.c:12: warning: implicit declaration of function `malloc'
fun.c:12: error: conversion to non-scalar type requested
fun.c:14: error: conversion to non-scalar type requested
fun.c:16: warning: implicit declaration of function `free'
Terminated with exit code 1

struct NODE;

This forward declaration is not useful.

struct NODE {
char string[80];
struct NODE *next;
}

fun.c:9: error: two or more data types in declaration of `main'

You missed a semicolon here.

int main()

Prefer 'int main(void)'. "()" and "(void)" mean completely different
things in C.

{
struct NODE *first;

first = (struct NODE) malloc(sizeof(struct NODE));

fun.c:12: warning: implicit declaration of function `malloc'

malloc has not been declared, so using it causes it to be implicitly
declared to return 'int'. This is wrong, and causes the behavior to be
undefined.


fun.c:12: error: conversion to non-scalar type requested

You've also cast to the wrong type.

You should probably not cast the return from malloc. It doesn't do
anything useful, makes maintenance more difficult, and can hide errors.
If you had cast to the correct type, and the compiler had not bothered
to warn about the implicit declaration of malloc (it's not required to,
and some don't), then you'd have a silent case of undefined behavior.

Finally, consider using the comp.lang.c-approved malloc idiom:

p = malloc(N * sizeof(*p));

This is less error-prone and more self-maintaining than other idioms.

first->next = (struct NODE) malloc(sizeof(struct NODE));

All the same problems as before, plus undefined behavior (probably
causing a crash) if the first malloc fails.

free(first);

Memory leak. You never freed first->next, and now you have no way to do so.

return 0;
}

Is it necessary to malloc the struct 'first'?

I'm not sure I know what you mean. 'first' is a pointer, not a struct.
It has to be made to point to something if you want it to be useful. A
malloced object is one possible choice.

In the context of this example, you didn't need malloc at all. You could
have done this:

struct NODE first, next;
first.next = next;

-Kevin

 

[Barry Schwarz]

I'm sorry for asking such a silly question, but I can't quite get my head
around malloc. Using gcc I have always programmed in a lax C/C++ hybrid
(which I suppose is actually c++). But I have started messing around in Plan
9, and that sort of thing is totally no go there .

Is this correct to allocate memory for my struct? It works on my computer,
but I'm suspicious that I'm doing it wrong.

--

You shouldn't use this separator. Many news readers treat everything
below as signature material.

struct NODE;

struct NODE {
char string[80];
struct NODE *next;
}
;


int main()
{
struct NODE *first;

first = (struct NODE) malloc(sizeof(struct NODE));

This is a constraint violation. You probably meant the cast to be
(struct NODE*).

In C you should never cast the return from malloc. You should also
#include stdlib.h to insure a prototype is in scope. The preferred C
construct is

first = malloc(sizeof *first)

first->next = (struct NODE) malloc(sizeof(struct NODE));

You ought to compile your code before submitting it.

free(first);

You do realize that this creates a memory leak?

return 0;
}

Is it necessary to malloc the struct 'first'?

If you don't, will first->next even exist?


<<Remove the del for email>>

 

[Malcolm]

I'm sorry for asking such a silly question, but I can't quite get my head
around malloc. Using gcc I have always programmed in a lax C/C++
hybrid (which I suppose is actually c++).

It's C-like C++. Unless you really know what you doing this has the
disadvantages of C and C++, and the advantages of neither.

struct NODE;
Get rid of this.

struct NODE {
char string[80];
struct NODE *next;
}
OK.

int main()
{
struct NODE *first;

first = (struct NODE) malloc(sizeof(struct NODE));

You mean first = (struct NODE *) malloc( sizeof(struct NODE));
or you can just say first = malloc( sizeof(struct NODE));

this means that first now points to an area of memory big enough to contain
one struct NODE. ie 80 bytes plus a few for the pointer.

Now you should check that malloc() has succeeded.
if(first == NULL)
{
fprintf(stderr, "Out of memory\n");
exit(EXIT_FAILURE);
}

first->next = (struct NODE) malloc(sizeof(struct NODE));

Imagine we want to create a linked list of N nodes.

(temp is a struct NODE *)
temp = first;
for(i=0;i<N-1;i++)
{
temp->next = malloc(sizeof(NODE));
if(!temp->next)
{
fprintf(stderr, "Out of memory\n");
exit(EXIT_FAILURE);
}
strcpy(temp->next->string, "Empty string");
temp ->next->next = 0;
temp = temp->next;
}

free(first);

You must free memory you allocate, in this case both first and first->next.
If you free first then you will orphan first->next.
Generally, when freeing a linked list, free the last member first and then
work upwards.

return 0;
}

Is it necessary to malloc the struct 'first'?

You can create a structure on the stack.

struct NODE first.

If you know how many structures you need in the linked list, you can create
an array

struct NODE list[100].
and then set the nest pointers.

However if the list can contain an arbitrary number of members, then you
need to allocate dynamically.

 

[Richard Hunt]

Sorry everyone, I wrote this example out quickly from memory without any
thought, before I went out. Your advice was useful though, thanks for trying
.

 

[Richard Hunt]

Here's a modified version of the program. I realize that it isn't a sensible
program, sorry. Does this look better?

#include <stdlib.h>

struct NODE;

/*
* This was why I had the forward declaration in the original, sorry
*/

struct interior {
char text[100];
struct NODE *next;
};

struct NODE {
char text[100];
struct interior yes;
struct interior no;
};

int main(int argc, char **argv)
{
struct NODE *first;

first=malloc(sizeof(struct NODE));

first->yes.next=malloc(sizeof(struct NODE));
first->no.next=malloc(sizeof(struct NODE));

/*
* free part skipped, see question
*/

return 0;
}

Assuming that I continue creating further nodes in this tree, what would be
the correct way to free them all? I can't work backwards, because there are
no pointers to earlier in the tree. The example I used for this said that I
could just do free(first). (It also claimed to be a C example, but used the
new keyword.)

I learnt what C I know originally from Illustrating ansi C, because it was
about, and I couldn't afford to get any more books. It isn't very
comprehensive, but I think it's accurate. Shame I lost it.

 

[Kevin Goodsell]

Here's a modified version of the program. I realize that it isn't a sensible
program, sorry. Does this look better?

#include <stdlib.h>

struct NODE;

/*
* This was why I had the forward declaration in the original, sorry
*/

struct interior {
char text[100];
struct NODE *next;
};

struct NODE {
char text[100];
struct interior yes;
struct interior no;
};

int main(int argc, char **argv)
{
struct NODE *first;

first=malloc(sizeof(struct NODE));

I would still recommend applying 'sizeof' to the object, not the type.
Otherwise you are sacrificing a large part of the benefit of the
clc-approved malloc idiom and making it more error-prone.

first = malloc(sizeof(*first));

OR

first = malloc(sizeof *first);

If you really want to give the type here (for some reason), I wouldn't
use the clc idiom at all. I'd probably do something completely
different, like the following:

#define ALLOC(type) (type *)malloc(sizeof(type))
/* ... */
first = ALLOC(struct NODE);

This will cause a diagnostic if you allocate space for the wrong type,
but still may suppress a useful diagnostic if you forget to #include

first->yes.next=malloc(sizeof(struct NODE));
first->no.next=malloc(sizeof(struct NODE));

/*
* free part skipped, see question
*/

return 0;
}

Assuming that I continue creating further nodes in this tree, what would be
the correct way to free them all? I can't work backwards, because there are
no pointers to earlier in the tree. The example I used for this said that I
could just do free(first). (It also claimed to be a C example, but used the
new keyword.)

Then the example was crap. You *must* free() everything you malloc() (or
realloc(), or calloc()). That means each malloc() invocation needs a
corresponding free() invocation (applied to a pointer with the same
value as the pointer returned from malloc()).

[OT: Also, even in C++ where the 'new' keyword exists, you cannot free()
memory allocated with 'new' - you have to use 'delete' with 'new' and
free() with malloc(). And even in C++, you have to match every
invocation of an allocation function (or operator) with an invocation of
the corresponding deallocation function (or operator). Neither C nor C++
require any sort of automatic clean-up of dynamically allocated memory
at any time, even at program termination.]

The way memory is usually freed for a tree is to do a regular old
post-order traversal, freeing as you go. Something like this:

void free_tree(NODE *ptr)
{
if (ptr)
{
free_tree(ptr->left_child);
free_tree(ptr->right_child);
free(ptr);
}
}

-Kevin

 

[Richard Hunt]

I would still recommend applying 'sizeof' to the object, not the type.

Otherwise you are sacrificing a large part of the benefit of the
clc-approved malloc idiom and making it more error-prone.

first = malloc(sizeof(*first));

OR

first = malloc(sizeof *first);

Sorry, I hadn't taken in what you said before, I was too busy trying to come
up with a correct example

[OT: Also, even in C++ where the 'new' keyword exists, you cannot free()
memory allocated with 'new' - you have to use 'delete' with 'new' and
free() with malloc(). And even in C++, you have to match every
invocation of an allocation function (or operator) with an invocation of
the corresponding deallocation function (or operator). Neither C nor C++
require any sort of automatic clean-up of dynamically allocated memory
at any time, even at program termination.]

Ah, free() instead of delete was probably my memory failing me. But my basic
point was definitely what the example said.

The way memory is usually freed for a tree is to do a regular old
post-order traversal, freeing as you go. Something like this:

void free_tree(NODE *ptr)
{
if (ptr)
{
free_tree(ptr->left_child);
free_tree(ptr->right_child);
free(ptr);
}
}

Thanks, thats what I wanted. I think I should invest in some decent books,
it's alright me soldiering on slowly in private, but I don't want to waste
any more of other peoples time.

 

[CBFalconer]

.... snip ...

Assuming that I continue creating further nodes in this tree,
what would be the correct way to free them all? I can't work
backwards, because there are no pointers to earlier in the tree.
The example I used for this said that I could just do free(first).
(It also claimed to be a C example, but used the new keyword.)

Study this elementary example:

#include <stdlib.h>

#define ITEMS 10

struct node {
struct node *next;
int datum;
};

int main(void)
{
struct node *root, *temp;;
int i; /* just a counter */

root = NULL;
for (i = 0; i < ITEMS; i++) {
temp = malloc(sizeof *temp);
if (temp) /* i.e temp != NULL */ {
temp->next = root; /* save previous */
root = temp; /* revise root value */
}
else {
/* malloc failed, bail out, temp == NULL */
return EXIT_FAILURE;
}
}
/* now we have allocated 10 nodes, the last of which
is pointed to by root, and each nodes next pointer
points to an earlier node, or is NULL at the end */

/* free them all */
while {root) /* i.e. while (root != NULL) */
temp = root->next; /* won't be available after free */
free(root);
root = temp; /* and see if at the end */
}
return 0;
} /* main untested */

Note that datum has never been used. Most of the comments are not
normally necessary, but I added them to clarify what things were
about to you.

After that is perfectly clear, see if you can create a binary
tree. Then consider ways to walk it, known as preorder, inorder,
and postorder. The sort of node you will want will be something
like:

struct node {
struct node *left, *right;
int datum;
};

Draw pictures with boxes representing nodes and lines representing
pointers. Don't forget that NULL points nowhere.

 

[Richard Bos]

Prefer 'int main(void)'. "()" and "(void)" mean completely different
things in C.

No, they don't. This is a function _definition_, and in a function
definition sometype function() and sometype function(void) mean exactly
the same thing: function() is a function taking no arguments and
returning a value of type sometype.
What you say _is_ true for function declarations:

sometype function(void);

means: I have, somewhere, defined a function called function(), taking
no arguments, and returning a sometype; but

sometype function();

means: I have, somewhere, defined a function called function(),
returning a sometype, and I'm not telling you what kind of arguments it
takes.
But this is only true for declarations, not for definitions.

Richard

 

[Richard Bos]

You shouldn't use this separator. Many news readers treat everything
below as signature material.

Then many newsreaders are broken[1]. A sigsep is
dash-dash-SPACE-newline.

Richard

[1] I know, I know... get a real newsreader, anyway!

 

[Kevin Goodsell]

No, they don't. This is a function _definition_, and in a function
definition sometype function() and sometype function(void) mean exactly
the same thing: function() is a function taking no arguments and
returning a value of type sometype.
What you say _is_ true for function declarations:

sometype function(void);

means: I have, somewhere, defined a function called function(), taking
no arguments, and returning a sometype; but

sometype function();

means: I have, somewhere, defined a function called function(),
returning a sometype, and I'm not telling you what kind of arguments it
takes.
But this is only true for declarations, not for definitions.

While discussing this exact issue, somebody pointed out to me a while
back that

int main()
{
/*...*/
}

is both a definition *and* a declaration. Maybe I'm wrong (it's much too
late for me to go looking through the standard for a precise answer),
but my guess would be that, in the absence of a declaration that
provides more information (i.e., a prototype), the rules of a
declaration apply to the definition. Consider this:

int func()
{
return 1;
}

int main(void)
{
func(1);
return 0;
}

gcc (-W -Wall -ansi -pedantic) accepts it with no complaints. Comeau
accepts it with only a warning. The same happens with this code:

int main()
{
main(1.2, 23, "huh?");
return 0;
}

I don't know exactly what the deal is, but if 'main()' is supposed to be
equivalent to 'main(void)' then both of these compilers are broken in
this respect.

-Kevin

 

[Alexander Bartolich]

begin followup to Richard Bos:

[...] in a function definition sometype function() and
sometype function(void) mean exactly the same thing:

$ cat a.c
#include <stdio.h>
int foo() { return puts("foo"); }
int main() { return foo(123);

$ gcc -Wall -ansi -pedantic a.c && ./a.out
foo

Repeat the thing with "foo(void)" und the compiler will complain:

a.c: In function `main':
a.c:3: too many arguments to function `foo'

[...] But this is only true for declarations, not for definitions.

You are proposing a magic difference in the meaning of signatures
between prototype and function body. Well, at least one compiler
knows nothing of that. And then I will offer you an alternative
explanation requiring no magic at all.

You can define functions with as many or few arguments as you like.
The caller of the function is free to pass as many or as few
arguments as he likes. This will work for two reasons:

1. The linker knows only the function name, nothing else.
2. Under the C calling convention the caller of the function is
responsible for any required disposal of function arguments
previously pushed on the stack.

So for all practical purposes a function is not required to know
how many arguments it got, be it "int" or "int, char**" or even
"int, char**, char**".

 

[Peter Nilsson]

No, they don't. This is a function _definition_, and in a function
definition sometype function() and sometype function(void) mean exactly
the same thing: function() is a function taking no arguments and
returning a value of type sometype.

Nonetheless, () is not a prototype, so a subtle difference is that the
following does not require a diagnostic...

int main()
{
main(0, (char **) 0);
}

That said, (void) is also preferable for the simple reason that () style
parameter lists are (still!) deprecated.

 

[Peter Nilsson]

begin followup to Richard Bos:

[...] in a function definition sometype function() and
sometype function(void) mean exactly the same thing:

$ cat a.c
#include <stdio.h>
int foo() { return puts("foo"); }
int main() { return foo(123);

$ gcc -Wall -ansi -pedantic a.c && ./a.out
foo

I must have an old gcc port...

% type a.c
#include <stdio.h>
int foo() { return puts("foo"); }
int main() { return foo(123);

% gcc -Wall -ansi -pedantic a.c
a.c: In function `main':
a.c:4: parse error at end of input

%

Hope it wasn't your news client, that would be ironic.

Repeat the thing with "foo(void)" und the compiler will complain:

a.c: In function `main':
a.c:3: too many arguments to function `foo'

Missing } aside, in that circumstance, a diagnostic is required as it is a
constraint violation.

[...] But this is only true for declarations, not for definitions.

You are proposing a magic difference in the meaning of signatures
between prototype and function body. Well, at least one compiler
knows nothing of that. And then I will offer you an alternative
explanation requiring no magic at all.

[snip]

The behaviour of your (corrected) program is undefined in either case.

C89 draft 3.2.2.2:

"...If the number of arguments does not agree with the number of
parameters, the behavior is undefined."

C's () is not equivalent to C++'s (...).

 

[Richard Bos]

While discussing this exact issue, somebody pointed out to me a while
back that

int main()
{
/*...*/
}

is both a definition *and* a declaration. Maybe I'm wrong (it's much too
late for me to go looking through the standard for a precise answer),
but my guess would be that, in the absence of a declaration that
provides more information (i.e., a prototype), the rules of a
declaration apply to the definition.

Hmmm... that _does_ sound logical, but it goes counter to what I've
always believed about function definitions.
_Inside_ the definition, of course, they are equivalent no matter what,
since there's no way to get at any arguments that may be provided.

Richard

 

[Richard Bos]

This will be out of topic, but I have something to say regarding the
casting in malloc.

Though the usual standards say that the casting in malloc is not
neccessary, I have found programs crashing with SIGILL, SIGBUS, and
SIGSEGV in my Solaris environment. This behaviour is when I use the 64
bit FORTE compiler to compile my code.

Let me guess: you tend to forget to #include <stdlib.h>?

Richard

 

[imemyself]

Hi,

This will be out of topic, but I have something to say regarding the
casting in malloc.

Though the usual standards say that the casting in malloc is not
neccessary, I have found programs crashing with SIGILL, SIGBUS, and
SIGSEGV in my Solaris environment. This behaviour is when I use the 64
bit FORTE compiler to compile my code.

In my case, if the void pointer returned from Malloc is not casted to
the one assigned to, then the program crashes under high(sometimes
low) virtual memory consumption. Casting the void pointers have solved
lot of problems for me.

Strange thing is not all the void pointers I have not casted gives me
problems.
Its random.

Regards
Vishal

I'm sorry for asking such a silly question, but I can't quite get my head
around malloc. Using gcc I have always programmed in a lax C/C++ hybrid
(which I suppose is actually c++). But I have started messing around in Plan
9, and that sort of thing is totally no go there .

Is this correct to allocate memory for my struct? It works on my computer,
but I'm suspicious that I'm doing it wrong.

No, it's not correct. It shouldn't even compile. Here are the
diagnostics I got:

"C:\cygwin\bin\gcc.exe" -W -Wall -Wcast-align -Wwrite-strings
-Wno-sign-compare -Wno-unused-parameter -Wpointer-arith -ansi
-pedantic-errors -ggdb -c fun.c
fun.c:9: error: two or more data types in declaration of `main'
fun.c: In function `main':
fun.c:12: warning: implicit declaration of function `malloc'
fun.c:12: error: conversion to non-scalar type requested
fun.c:14: error: conversion to non-scalar type requested
fun.c:16: warning: implicit declaration of function `free'
Terminated with exit code 1

struct NODE;

This forward declaration is not useful.

struct NODE {
char string[80];
struct NODE *next;
}

fun.c:9: error: two or more data types in declaration of `main'

You missed a semicolon here.

int main()

Prefer 'int main(void)'. "()" and "(void)" mean completely different
things in C.

{
struct NODE *first;

first = (struct NODE) malloc(sizeof(struct NODE));

fun.c:12: warning: implicit declaration of function `malloc'

malloc has not been declared, so using it causes it to be implicitly
declared to return 'int'. This is wrong, and causes the behavior to be
undefined.


fun.c:12: error: conversion to non-scalar type requested

You've also cast to the wrong type.

You should probably not cast the return from malloc. It doesn't do
anything useful, makes maintenance more difficult, and can hide errors.
If you had cast to the correct type, and the compiler had not bothered
to warn about the implicit declaration of malloc (it's not required to,
and some don't), then you'd have a silent case of undefined behavior.

Finally, consider using the comp.lang.c-approved malloc idiom:

p = malloc(N * sizeof(*p));

This is less error-prone and more self-maintaining than other idioms.

first->next = (struct NODE) malloc(sizeof(struct NODE));

All the same problems as before, plus undefined behavior (probably
causing a crash) if the first malloc fails.

free(first);

Memory leak. You never freed first->next, and now you have no way to do so.

return 0;
}

Is it necessary to malloc the struct 'first'?

I'm not sure I know what you mean. 'first' is a pointer, not a struct.
It has to be made to point to something if you want it to be useful. A
malloced object is one possible choice.

In the context of this example, you didn't need malloc at all. You could
have done this:

struct NODE first, next;
first.next = next;

-Kevin

 

[Alexander Bartolich]

begin followup to Peter Nilsson:

Hope it wasn't your news client, that would be ironic.

Definitely not. Copy-n-paste fuckup.
My problem exists between mouse and chair.
But then I had to say that.
Sacrify ego or put blame on holy slrn?
For the true believer a no-brainer...

C89 draft 3.2.2.2:

"...If the number of arguments does not agree with the
number of parameters, the behavior is undefined."

Well, yes. The holy book. I guess that in there is also a special
rule about the many faces of main.

main(int,char**)
main(int,char**,char**)
main()
main(...)
main(void)

Ok, ok, that's heresy. Nowhere in the standard is written that
implementations of C follow the principle of logic and least
complicated design.

Stating that main is not a special case but just one instance
of a general principle might get bonus for scientific attitude,
but sure damnation by the high priests of ANSI.

Ok, ok, ANSI is always right, while my parody of a scientific
thesis breaks on a significant number of implementations.
For example on "gcc -mrtd".

The Hitchhikers Guide to the GCC has this to say about -mrtd:
# Use a different function-calling convention, in which functions
# that take a fixed number of arguments return with the `ret' NUM
# instruction, which pops their arguments while returning. This
# saves one instruction in the caller since there is no need to pop
# the arguments there.

C's () is not equivalent to C++'s (...).

Well, let's say that on the same platform they both will have
the same technical problem. In this way they are very equivalent.