Looking for malloc() help

[SP]

I am learning C and have a question re: malloc().

I wrote simple program which assigns a value to a structure and then
prints it as follow:

#include <stdio.h>
#include <stdlib.h>

struct item {
char name[20];
int quantity;
};

int main (int argc, char *argv[])
{
struct item *stuff;

//allocate memory for structure
stuff = malloc(3 * sizeof(struct item));

strcpy(stuff[1].name, "apple");
stuff[1].quantity = 1;
strcpy(stuff[2].name, "banana");
stuff[2].quantity = 2;

printf("%s %d\n", stuff[1].name, st.quantity);
printf("%s %d\n", stuff[2].name, st.quantity);

free(stuff);
return 0;
}

I then change the structure declaration in order to dynamically
allocate memory
for the char array:
struct item {
char *name;
int quantity;
};

The program compiles with no errors, but it crashes with the following
error:

line 3: 2485 Segmentation fault

Thanks for your help

[xiaohuamao]

I don't know what is the problem. The following code works in my
machine.
struct item {
char* name;
int quantity;
};

int main ()
{
struct item *stuff;

//allocate memory for structure
stuff = (struct item *)malloc(3 * sizeof(struct item));

stuff[1].name = (char *) malloc(sizeof(" apple"));
strcpy(stuff[1].name, "apple");
stuff[1].quantity = 1;
stuff[2].name = (char *) malloc(sizeof(" banana"));
strcpy(stuff[2].name, "banana");
stuff[2].quantity = 2;
printf("%s %d\n", stuff[1].name, stuff[1].quantity);
printf("%s %d\n", stuff[2].name, stuff[2].quantity);
free(stuff);
return 0;
}

[SP]

xiaohuamao wrote:

I don't know what is the problem. The following code works in my
machine.
struct item {
char* name;
int quantity;
};

int main ()
{
struct item *stuff;

//allocate memory for structure
stuff = (struct item *)malloc(3 * sizeof(struct item));

stuff[1].name = (char *) malloc(sizeof(" apple"));
strcpy(stuff[1].name, "apple");
stuff[1].quantity = 1;
stuff[2].name = (char *) malloc(sizeof(" banana"));
strcpy(stuff[2].name, "banana");
stuff[2].quantity = 2;
printf("%s %d\n", stuff[1].name, stuff[1].quantity);
printf("%s %d\n", stuff[2].name, stuff[2].quantity);
free(stuff);
return 0;
}

Not sure it matters, my PC is as follows:

Linux Slackware 10.1
compiling program with "gcc -o test struct.c"

Is anyone else able to run this ?

[Thomas Lumley]

SP wrote:

I am learning C and have a question re: malloc().

I wrote simple program which assigns a value to a structure and then
prints it as follow:

#include <stdio.h>
#include <stdlib.h>

struct item {
char name[20];
int quantity;
};

int main (int argc, char *argv[])
{
struct item *stuff;

//allocate memory for structure
stuff = malloc(3 * sizeof(struct item));

strcpy(stuff[1].name, "apple");
stuff[1].quantity = 1;
strcpy(stuff[2].name, "banana");
stuff[2].quantity = 2;

printf("%s %d\n", stuff[1].name, st.quantity);

st has not been defined. You probably mean stuff[1].quantity

printf("%s %d\n", stuff[2].name, st.quantity);

stuff[2].quantity

>
free(stuff);
return 0;
}
I then change the structure declaration in order to dynamically
allocate memory
for the char array:
struct item {
char *name;
int quantity;
};

The program compiles with no errors, but it crashes with the following
error:

In the first case the struct contains memory (20 bytes) for storing the
names so the malloc() for the structs is sufficient. In the second
case the struct contains only a pointer, so you are copying "apple" to
whatever random address ends up in stuff[1].name. Undefined behaviour.[*]

You could either use malloc() to allocate space for storing the name
and assign the resulting pointer to stuff[1].name, or just do
stuff[1].name = "apple";
and not copy the string at all.

In this case just assigning the pointer would make more sense, but in a
real example either strategy might be appropriate.

-thomas

[*] Technically the undefined behaviour could be even worse than
copying to some random address, but that's bad enough.

[Keith Thompson]

"SP" <po******@susco m.netwrites:

I am learning C and have a question re: malloc().

I wrote simple program which assigns a value to a structure and then
prints it as follow:

#include <stdio.h>
#include <stdlib.h>

struct item {
char name[20];
int quantity;
};

int main (int argc, char *argv[])
{
struct item *stuff;

//allocate memory for structure
stuff = malloc(3 * sizeof(struct item));

strcpy(stuff[1].name, "apple");
stuff[1].quantity = 1;
strcpy(stuff[2].name, "banana");
stuff[2].quantity = 2;

printf("%s %d\n", stuff[1].name, st.quantity);
printf("%s %d\n", stuff[2].name, st.quantity);

free(stuff);
return 0;
}

I then change the structure declaration in order to dynamically
allocate memory
for the char array:
struct item {
char *name;
int quantity;
};

The program compiles with no errors, but it crashes with the following
error:

line 3: 2485 Segmentation fault

So you've shown us the program that works, but not the one that
doesn't.

Ok, I'll assume that the change in the declaration of "struct item"
was the only change you made. The problem is that you haven't
allocated space for the strings "apple" and "banana". You need
to do something like:

stuff[1].name = malloc(some_num ber_of_bytes);
strcpy(stuff[1].name, "apple");

Some other notes:

If you don't use argc and argv, you can omit their declarations:
int main(void)

Always check the result of malloc(). If it fails for whatever reason,
you go on and try to access the memory anyway; this can Make Bad
Things Happen. Even if you just abort the program on failure, it's
better than ignoring it.

Your malloc() call is good, but there's a little trick that can make
it even better:

stuff = malloc(3 * sizeof *stuff);

This way, you don't have to repeat (and possibly get wrong) the type
of "stuff" in the malloc call, and if you change the type of stuff you
don't have to track down and change all the calls.

I presume you know you're using only the 2nd and 3rd of the 3 elements
you allocated for your array.

It's generally considered a good idea to avoid "//" comments when
posting to Usenet. News software often wraps long lines. If a "//"
comment is wrapped, it usually creates a syntax error; if a "/*
.... */" comment is wrapped, it's usually harmless. (And "//" comments
aren't supported in C90.)

--
Keith Thompson (The_Other_Keit h) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Richard Heathfield]

SP said:

I am learning C and have a question re: malloc().

I wrote simple program which assigns a value to a structure and then
prints it as follow:

#include <stdio.h>
#include <stdlib.h>

struct item {
char name[20];
int quantity;
};

int main (int argc, char *argv[])
{
struct item *stuff;

//allocate memory for structure
stuff = malloc(3 * sizeof(struct item));

Not bad, but better would be:

stuff = malloc(3 * sizeof *stuff); /* note the * in front of stuff */

because there's less to get wrong (you don't need to type the typename, so
you can't mess it up - not that you did, on this occasion), and it's more
robust in the face of subsequent changes to the pointer type.

But malloc can fail. In that eventuality, it will return NULL. You should
check for this before relying on the allocation.

For a "student exercise" such as this one, the following would be
sufficient:

if(stuff == NULL)
{
fputs("Insuffic ient memory to continue. Terminating program.\n",
stderr);
exit(EXIT_FAILU RE);
}

>
strcpy(stuff[1].name, "apple");
stuff[1].quantity = 1;

You're not using stuff[0], then? Well, okay, it's your memory...

strcpy(stuff[2].name, "banana");
stuff[2].quantity = 2;

printf("%s %d\n", stuff[1].name, st.quantity);
printf("%s %d\n", stuff[2].name, st.quantity);

free(stuff);
return 0;
}

That's fine, apart from what I've pointed out already.

>
I then change the structure declaration in order to dynamically
allocate memory
for the char array:
struct item {
char *name;
int quantity;
};

The program compiles with no errors, but it crashes with the following
error:

line 3: 2485 Segmentation fault

Yes. Now that you've changed name from being an array to being a mere
pointer, you need to point it at some storage.

if(stuff != NULL)
{
int i;
for(i = 0; i < 3; i++)
{
stuff[i].name = malloc(20 * sizeof *stuff[i].name);
if(stuff[i].name == NULL)
{
fprintf(stderr, "Can't allocate space for name %d\n", i);
fprintf(stderr, "Terminatin g program.\n");
exit(EXIT_FAILU RE);
}
}

You will now be able to do your strcpy in safety (provided the string you
copy into it is 19 or fewer bytes long).

Once you've finished, and before you free(stuff), you should do this:

for(i = 0; i < 3; i++)
{
free(stuff[i].name);
}

Then you can free(stuff);

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

[Richard Heathfield]

Thomas Lumley said:

>
SP wrote:

<snip>

>>
printf("%s %d\n", stuff[1].name, st.quantity);

st has not been defined. You probably mean stuff[1].quantity

> printf("%s %d\n", stuff[2].name, st.quantity);

Well spotted. I missed those completely.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

[SP]

Richard Heathfield wrote:

Thomas Lumley said:

SP wrote:

<snip>

>
printf("%s %d\n", stuff[1].name, st.quantity);

st has not been defined. You probably mean stuff[1].quantity

printf("%s %d\n", stuff[2].name, st.quantity);

Well spotted. I missed those completely.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

Allocating memory for the char array was the part I didnt know how to
do,
now it works just fine.

Thanks to all.

[Herbert Rosenau]

On Thu, 3 Aug 2006 03:02:20 UTC, "xiaohuamao "
<ca************ **@gmail.comwro te:

I don't know what is the problem. The following code works in my
machine.
struct item {
char* name;
int quantity;
};

int main ()
{
struct item *stuff;

//allocate memory for structure
stuff = (struct item *)malloc(3 * sizeof(struct item));

Never ever cast the result from malloc() except you like to go into
the lands of undefined behavior and you dont like to get diagnostics
from your compiler when you've forget to include stdlib.h.

Never ever cast something - excet you knows exactly what you're
doing. casting the result of a function returning void* is playing
with the health of y<or app because you does NOT know what you does.
When you would know what you does you would never cast that.

stuff[1].name = (char *) malloc(sizeof(" apple"));

see above

strcpy(stuff[1].name, "apple");
stuff[1].quantity = 1;
stuff[2].name = (char *) malloc(sizeof(" banana"));

see aboove

strcpy(stuff[2].name, "banana");
stuff[2].quantity = 2;
printf("%s %d\n", stuff[1].name, stuff[1].quantity);
printf("%s %d\n", stuff[2].name, stuff[2].quantity);
free(stuff);

Before you frees stuff you should free its members you've allocated
with malloc() to avoid memory leaks.

return 0;
}

Array starts with index 0, not 1. Why does you malloc 3 struct members
when you only needs 2? And why does you use the last but not the first
ones?

--
Tschau/Bye
Herbert

Visit http://www.ecomstation.de the home of german eComStation
eComStation 1.2 Deutsch ist da!

[Herbert Rosenau]

On Thu, 3 Aug 2006 12:09:24 UTC, "SP" <po******@susco m.netwrote:

>
Richard Heathfield wrote:

Thomas Lumley said:

>
SP wrote:

<snip>

>>
> printf("%s %d\n", stuff[1].name, st.quantity);
st has not been defined. You probably mean stuff[1].quantity
> printf("%s %d\n", stuff[2].name, st.quantity);

Well spotted. I missed those completely.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

Allocating memory for the char array was the part I didnt know how to
do,
now it works just fine.

No, you've forgotten to include stdlib.h so even as it seems to work
you're sitting in the middle of the lands of undefined behavior.

Thanks to all.

--
Tschau/Bye
Herbert

Visit http://www.ecomstation.de the home of german eComStation
eComStation 1.2 Deutsch ist da!