Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > malloc trouble

Reply
Thread Tools

malloc trouble

 
 
ncf
Guest
Posts: n/a
 
      10-07-2005
Hi all.

In another topic, I was informed that I had to dynamically allocate
memory instead of just trying to expand on a list. (I'm trying to learn
C, and have a strong background in PHP and Python) In light of that, I
have been trying to learn malloc, realloc, and free, but to no avail.

But for some reason, I'm getting segfaults right and left, and to be
honest, I am not having any luck at all really in finding out why it
isn't working. However, I have traced it and discovered that the
malloc() call is what is causing the problem. (traced by adding various
printf()s and then moving the malloc to it's own line)

It would be greatly appreciated if anyone can point out what the heck
I'm doing so wrong. Code snipplets are included at the end of this
message.

Thank you soo much in advance.

-Wes



/* at the "file" level */
#define BUFFLEN 100
char **messages;
int num_messages=0;
/* inside of main() */
if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN))
==NULL)
{
printf("Could not allocate space.\n");
return 1;
}

 
Reply With Quote
 
 
 
 
Artie Gold
Guest
Posts: n/a
 
      10-07-2005
ncf wrote:
> Hi all.
>
> In another topic, I was informed that I had to dynamically allocate
> memory instead of just trying to expand on a list. (I'm trying to learn
> C, and have a strong background in PHP and Python) In light of that, I
> have been trying to learn malloc, realloc, and free, but to no avail.
>
> But for some reason, I'm getting segfaults right and left, and to be
> honest, I am not having any luck at all really in finding out why it
> isn't working. However, I have traced it and discovered that the
> malloc() call is what is causing the problem. (traced by adding various
> printf()s and then moving the malloc to it's own line)
>
> It would be greatly appreciated if anyone can point out what the heck
> I'm doing so wrong. Code snipplets are included at the end of this
> message.
>
> Thank you soo much in advance.
>
> -Wes
>
>
>
> /* at the "file" level */
> #define BUFFLEN 100
> char **messages;


[In the future post *real* *compilable* code. It makes things easier.]

What's the value of `messages' at his point?

> int num_messages=0;
> /* inside of main() */
> if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN))
> ==NULL)
> {
> printf("Could not allocate space.\n");
> return 1;
> }
>

Please see the FAQ, particularly the piece on `dynamic allocation of
multidimensional arrays'.

HTH,
--ag

--
Artie Gold -- Austin, Texas
http://goldsays.blogspot.com (new post 8/5)
http://www.cafepress.com/goldsays
"If you have nothing to hide, you're not trying!"
 
Reply With Quote
 
 
 
 
ncf
Guest
Posts: n/a
 
      10-07-2005
Alrighty, I just checked one comp.lang.c faq (located at
http://www.faqs.org/faqs/C-faq/faq/), and to be quite honest, the code
they're using is making little sense to me, but I will give it a shot.

And, even though it may or may not be necessary until I get the chance
to add the pointer allocation, the code is currently as follows:


/*
* malloc/realloc/free test
*/
#include <string.h>
#include <stdio.h>
#include <stdlib.h>

#define BUFFLEN 10

char **messages;
int num_messages=0;

int main(int argc, char **argv) {
char tmpmsg[][BUFFLEN] = {"a","b","c","d","e"};
int tmpnum = sizeof(tmpmsg)/BUFFLEN;

printf("Going to copy %d messages.\n",tmpnum);

/* Do the copy loop */
int x;
for (x=0; x<tmpnum; x++) {
printf("Allocating space for %d.\n",x);
if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN))
==NULL)
{
printf("Could not allocate space.\n");
return 1;
}

strcpy(messages[num_messages], tmpmsg[x]);
num_messages++;
}

/* print all the values */
for (x=0;x<num_messages;x++) {
printf("%02d => %s\n",x,messages[x]);
}

/* clear and free all the positions of memory */
for (x=num_messages-1;x>=0;x--) {
memset(messages[x], '\0', sizeof(messages[x]));
free(messages[x]);
}

/* the end of the test...finally */
return 0;
}

 
Reply With Quote
 
ncf
Guest
Posts: n/a
 
      10-07-2005
Ok, wow, I think I get it now, but it would be grealy appreciated if
someone could look over my updated/modified code to make sure that it
does as it seems it should. Thank you for pointing out the obvious
(faq)--some days I deserve to be slapped

-Wes



/*
* malloc/realloc/free test
*/
#include <string.h>
#include <stdio.h>
#include <stdlib.h>

#define BUFFLEN 10

char **messages;
int num_messages=0;

int main(int argc, char **argv) {
char tmpmsg[][BUFFLEN] = {"a","b","c","d","e"};
int tmpnum = sizeof(tmpmsg)/BUFFLEN;

printf("Going to copy %d messages.\n",tmpnum);

/* first, malloc 0*sizeof(char *) bytes for messages, as we have no
pointers yet */
messages = malloc(num_messages*sizeof(char *));

/* Do the copy loop */
int x;
for (x=0; x<tmpnum; x++) {
printf("Allocating space for %d.\n",x);
if ( (messages = realloc(messages, (num_messages+1)*sizeof(char *)))
== NULL)
{
printf("Could not allocate space for one more pointer.\n");
return 1;
}
if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN))
==NULL)
{
printf("Could not allocate space.\n");
return 1;
}

strcpy(messages[num_messages], tmpmsg[x]);
num_messages++;
}

/* print all the values */
for (x=0;x<num_messages;x++) {
printf("%02d => %s\n",x,messages[x]);
}

/* clear and free all the positions of memory */
for (x=num_messages-1;x>=0;x--) {
memset(messages[x], '\0', sizeof(messages[x]));
free(messages[x]);
}
free(messages);

/* the end of the test...finally */
return 0;
}

 
Reply With Quote
 
Gordon Burditt
Guest
Posts: n/a
 
      10-07-2005
>But for some reason, I'm getting segfaults right and left, and to be
>honest, I am not having any luck at all really in finding out why it
>isn't working. However, I have traced it and discovered that the
>malloc() call is what is causing the problem. (traced by adding various
>printf()s and then moving the malloc to it's own line)
>
>It would be greatly appreciated if anyone can point out what the heck
>I'm doing so wrong. Code snipplets are included at the end of this
>message.
>
>Thank you soo much in advance.
>
>-Wes
>
>
>
>/* at the "file" level */
>#define BUFFLEN 100
>char **messages;
>int num_messages=0;
>/* inside of main() */


You may not assign to messages[num_messages] until you
have assigned something to messages (and something other
than NULL), which you don't show here.

> if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN))
>==NULL)
> {
> printf("Could not allocate space.\n");
> return 1;
> }


Gordon L. Burditt
 
Reply With Quote
 
Artie Gold
Guest
Posts: n/a
 
      10-07-2005
ncf wrote:
> Ok, wow, I think I get it now, but it would be grealy appreciated if
> someone could look over my updated/modified code to make sure that it
> does as it seems it should. Thank you for pointing out the obvious
> (faq)--some days I deserve to be slapped
>
> -Wes
>

You're obviously new around here. *That* was no slap!
>
>
> /*
> * malloc/realloc/free test
> */
> #include <string.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> #define BUFFLEN 10
>
> char **messages;
> int num_messages=0;
>
> int main(int argc, char **argv) {
> char tmpmsg[][BUFFLEN] = {"a","b","c","d","e"};
> int tmpnum = sizeof(tmpmsg)/BUFFLEN;
>
> printf("Going to copy %d messages.\n",tmpnum);
>
> /* first, malloc 0*sizeof(char *) bytes for messages, as we have no
> pointers yet */
> messages = malloc(num_messages*sizeof(char *));


Why bother? Leave this line out.
>
> /* Do the copy loop */
> int x;
> for (x=0; x<tmpnum; x++) {
> printf("Allocating space for %d.\n",x);
> if ( (messages = realloc(messages, (num_messages+1)*sizeof(char *)))
> == NULL)


Though it's not applicable here (as receiving a NULL from realloc will
lead to an immediate exit, as per the code below), you should assign the
return value of realloc() to a temporary variable, just in case it
fails. Otherwise (assuming your program can continue) you've created a
memory leak as you've lost the pointer to the original buffer.

Also, instead of using `sizeof (char *)' use `sizeof *messages'; it's a
style issue, to be sure (and not immediately important here) but it
helps in cases where the type you're allocating may change [it's always
preferable to have to make as few changes as possible when something,
well, changes.]

> {
> printf("Could not allocate space for one more pointer.\n");
> return 1;
> }
> if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN))
> ==NULL)

Don't cast the return value of malloc(). It's unnecessary and can hide
errors.
> {
> printf("Could not allocate space.\n");
> return 1;
> }
>
> strcpy(messages[num_messages], tmpmsg[x]);
> num_messages++;
> }
>
> /* print all the values */
> for (x=0;x<num_messages;x++) {
> printf("%02d => %s\n",x,messages[x]);
> }
>
> /* clear and free all the positions of memory */
> for (x=num_messages-1;x>=0;x--) {
> memset(messages[x], '\0', sizeof(messages[x]));


Why bother? You're just about to free the space anyway!

> free(messages[x]);
> }
> free(messages);
>
> /* the end of the test...finally */
> return 0;
> }
>

You're getting there.

Cheers and HTH,
--ag

--
Artie Gold -- Austin, Texas
http://goldsays.blogspot.com (new post 8/5)
http://www.cafepress.com/goldsays
"If you have nothing to hide, you're not trying!"
 
Reply With Quote
 
ncf
Guest
Posts: n/a
 
      10-07-2005
I definately chopped this up to shrink it down quite a bit, but yea, my
replies are inline.

Artie Gold wrote:
> > Thank you for pointing out the obvious (faq)--some days I deserve to
> > be slapped

> You're obviously new around here. *That* was no slap!

Hehe, no, but I practically slapped myself over some of the stupid
errors I've done.


> > messages = malloc(num_messages*sizeof(char *));

> Why bother? Leave this line out.

Hmm...how would I do the realloc later then if the space was never
alloc'd? Or would that be unnecessary as well? :slightly confused:


> > if ( (messages = realloc(messages, (num_messages+1)*sizeof(char *))) == NULL)

> Though it's not applicable here (as receiving a NULL from realloc will
> lead to an immediate exit, as per the code below), you should assign the
> return value of realloc() to a temporary variable, just in case it
> fails. Otherwise (assuming your program can continue) you've created a
> memory leak as you've lost the pointer to the original buffer.

Hmm...I *think* I see what you're saying. By memory leak, I'm infering
that you mean memory that was never free()d for other applications to
use.


> Also, instead of using `sizeof (char *)' use `sizeof *messages'; it's a
> style issue, to be sure (and not immediately important here) but it
> helps in cases where the type you're allocating may change [it's always
> preferable to have to make as few changes as possible when something,
> well, changes.]

Heh, alrighty I'll try to keep that in mind next time so you don't
kill me (JPJP)


> > if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN)) ==NULL)

> Don't cast the return value of malloc(). It's unnecessary and can hide
> errors.

Hmm...I'm not too sure right now how it'd hide errors, but hey! It'll
make sense probably later on, so lets not worry too much about that.
I'll just take your word for it


> > memset(messages[x], '\0', sizeof(messages[x]));

> Why bother? You're just about to free the space anyway!

Eh, I'm one of those odd people that likes to clean stuff up after
usage. But in retrospect, wasted CPU (however little).


> You're getting there.

Horray /Rand


Thanks AG and have a GREAT day
-Wes

 
Reply With Quote
 
Michael Mair
Guest
Posts: n/a
 
      10-07-2005
ncf wrote:
> Artie Gold wrote:
>>>messages = malloc(num_messages*sizeof(char *));

>>
>>Why bother? Leave this line out.

>
> Hmm...how would I do the realloc later then if the space was never
> alloc'd? Or would that be unnecessary as well? :slightly confused:


realloc(NULL, size) does the same thing as malloc(size),
realloc(ptr, 0) does the same thing as free(ptr).
Your implementation typically comes with a standard library
reference which you should look into; if not, try the C99
library reference on dinkumware.com

>>>if ( (messages = realloc(messages, (num_messages+1)*sizeof(char *))) == NULL)

>>
>>Though it's not applicable here (as receiving a NULL from realloc will
>>lead to an immediate exit, as per the code below), you should assign the
>>return value of realloc() to a temporary variable, just in case it
>>fails. Otherwise (assuming your program can continue) you've created a
>>memory leak as you've lost the pointer to the original buffer.

>
> Hmm...I *think* I see what you're saying. By memory leak, I'm infering
> that you mean memory that was never free()d for other applications to
> use.


Yes and no. It may be also memory _you_ cannot use later on in
your program because you already allocated but sort of lost the
key to use it. If you need large amounts of memory this may be
the bit which makes your program exit earlier than planned due
to (self-inflicted) lack of memory...
As an aside, most modern operating systems clean up after an
application finished, so many well-known applications accept
that there are memory leaks in their code that could not be
tracked down. This is bad practice and may hide other errors
which then come down as soon as some minor detail changes. Just
avoid them


>>>if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN)) ==NULL)

>>
>>Don't cast the return value of malloc(). It's unnecessary and can hide
>>errors.

>
> Hmm...I'm not too sure right now how it'd hide errors, but hey! It'll
> make sense probably later on, so lets not worry too much about that.
> I'll just take your word for it


For one, it is not necessary. The other thing is that casts
are pretty strong: They tell the compiler to shut up.
In combination with the "implicit int" rule, this may shadow
the fact that you forgot to #include <stdlib.h> which in turn
can lead to odd effects if sizeof(int) is different from the
size of the respective pointer type you cast to -- it may go
well 99% of the time but every now and then, you have your
program eating your hard disc's contents or similar...


>>You're getting there.

>
> Horray /Rand
>
> Thanks AG and have a GREAT day


My, you _really_ have promise


Cheers
Michael
--
E-Mail: Mine is an /at/ gmx /dot/ de address.
 
Reply With Quote
 
Niklas Norrthon
Guest
Posts: n/a
 
      10-07-2005
"ncf" <(E-Mail Removed)> writes:

> Ok, wow, I think I get it now, but it would be grealy appreciated if
> someone could look over my updated/modified code to make sure that it
> does as it seems it should. Thank you for pointing out the obvious
> (faq)--some days I deserve to be slapped


Here is my take: Your code is fine, only minor issues.

>
> -Wes
>
>
>
> /*
> * malloc/realloc/free test
> */
> #include <string.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> #define BUFFLEN 10
>
> char **messages;


char **messages = NULL; /* initalize, so we can use realloc from start */

> int num_messages=0;
>
> int main(int argc, char **argv) {


int main(void) { /* here we don't use command line arguments, so void
eliminates a couple of warnings */

> char tmpmsg[][BUFFLEN] = {"a","b","c","d","e"};
> int tmpnum = sizeof(tmpmsg)/BUFFLEN;
>
> printf("Going to copy %d messages.\n",tmpnum);
>
> /* first, malloc 0*sizeof(char *) bytes for messages, as we have no
> pointers yet */


/* Remove this:

> messages = malloc(num_messages*sizeof(char *));


*/

/*
* I (and many with me) prefer the style: ptr = malloc(count * sizeof *ptr);
* That works all the time, is always the same (you don't need to think of the
* type at all), but here you don't need malloc at all, since you don't need
* any memory yet, but just a defined starting point for later calls to realloc.
* Setting messages = NULL is enough. (Calling realloc with a NULL ptr is
* exactly the same as calling malloc).
*/



>
> /* Do the copy loop */


{ /* in C90 variable declarations must only appear before executable code
this has changed in C99, but must people still use C90 compilers */

> int x;
> for (x=0; x<tmpnum; x++) {
> printf("Allocating space for %d.\n",x);
> if ( (messages = realloc(messages, (num_messages+1)*sizeof(char *)))
> == NULL)


/* Style: */ messages = realloc(messages, num_messages + 1 * sizeof *messages);

/* Also notice that if you need somewhat more sofisiticated error handling
* you lose the original buffer if realloc fails. In that case you need to do
* something like:
* tmp = realloc(...);
* if (tmp == NULL) {
* error_handling(...);
* free(messages) /* or maybe not, depending on everything else */
* return, abort, exit or something perhaps
* }
* messages = tmp;
*/

> {
> printf("Could not allocate space for one more pointer.\n");
> return 1;


return EXIT_FAILURE; /* Only defined return codes from main are
0, EXIT_OK, and EXIT_FAILURE */

> }
> if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN))
> ==NULL)


/* Unnecessary casts, I'd say: */
if ((message[num_messages] = malloc(BUFFLEN)) == NULL)


> {
> printf("Could not allocate space.\n");
> return 1;


return EXIT_FAILURE;

> }
>
> strcpy(messages[num_messages], tmpmsg[x]);
> num_messages++;
> }
>
> /* print all the values */
> for (x=0;x<num_messages;x++) {
> printf("%02d => %s\n",x,messages[x]);
> }
>
> /* clear and free all the positions of memory */
> for (x=num_messages-1;x>=0;x--) {
> memset(messages[x], '\0', sizeof(messages[x]));
> free(messages[x]);
> }
> free(messages);


} /* end of local block for int x; */

>
> /* the end of the test...finally */
> return 0;
> }

 
Reply With Quote
 
Christopher Benson-Manica
Guest
Posts: n/a
 
      10-07-2005
ncf <(E-Mail Removed)> wrote:

> Alrighty, I just checked one comp.lang.c faq (located at
> http://www.faqs.org/faqs/C-faq/faq/), and to be quite honest, the code
> they're using is making little sense to me, but I will give it a shot.


It is proper Usenet etiquette to include the text you are replying to.
To do this using Google groups, please follow the instructions below,
penned by Keith Thompson:

If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers.

> if ( (messages[num_messages] = (char *)malloc((size_t)BUFFLEN))


It isn't in the FAQ, but should be: Casting the return of malloc() is
both unnecessary and inadvisable. Read this group's archives for
details.

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
to malloc or not to malloc?? Johs32 C Programming 4 03-30-2006 10:03 AM
porting non-malloc code to malloc micromysore@gmail.com C Programming 3 02-19-2005 05:39 AM
Malloc/Free - freeing memory allocated by malloc Peter C Programming 34 10-22-2004 10:23 AM
free'ing malloc'd structure with malloc'd members John C Programming 13 08-02-2004 11:45 AM
Re: free'ing malloc'd structure with malloc'd members ravi C Programming 0 07-30-2004 12:42 PM



Advertisments