Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Proper way to check malloc return

Reply
Thread Tools

Proper way to check malloc return

 
 
Billy Mays
Guest
Posts: n/a
 
      05-24-2010
I typically check my malloc calls like this:

if( !(pointer = malloc(number)) )
{
fprintf(stderr, "An error!\n");
exit(EXIT_FAILURE);
}

.... or something similar. However, I often see people do this:

pointer = malloc(number);
if( number == NULL )
{
fprintf(stderr, "An error!\n");
exit(EXIT_FAILURE);
}


Is there a proper way to check the return value of a malloc call? Are
there any benefits to doing it one way over another?

Bill
 
Reply With Quote
 
 
 
 
Tom St Denis
Guest
Posts: n/a
 
      05-24-2010
On May 24, 9:03*am, Billy Mays <(E-Mail Removed)> wrote:
> I typically check my malloc calls like this:
>
> if( !(pointer = malloc(number)) )
> {
> * * *fprintf(stderr, "An error!\n");
> * * *exit(EXIT_FAILURE);
>
> }
>
> ... or something similar. *However, I often see people do this:
>
> pointer = malloc(number);
> if( number == NULL )


I'm assuming this was supposed to be pointer == NULL ...

> {
> * * *fprintf(stderr, "An error!\n");
> * * *exit(EXIT_FAILURE);
>
> }
>
> Is there a proper way to check the return value of a malloc call? *Are
> there any benefits to doing it one way over another?


Well generally putting assignments inside other statements is
considered bad form (exceptions being the 'for' keyword).

I'd still check for NULL eitherway though

if ((pointer = malloc(number)) == NULL) {
... error
}

And not use ! on a pointer (though it's probably fine, I don't care to
check though).

Tom
 
Reply With Quote
 
 
 
 
Tim Streater
Guest
Posts: n/a
 
      05-24-2010
In article
<(E-Mail Removed)>,
Tom St Denis <(E-Mail Removed)> wrote:

> On May 24, 9:03*am, Billy Mays <(E-Mail Removed)> wrote:
> > I typically check my malloc calls like this:
> >
> > if( !(pointer = malloc(number)) )
> > {
> > * * *fprintf(stderr, "An error!\n");
> > * * *exit(EXIT_FAILURE);
> >
> > }
> >
> > ... or something similar. *However, I often see people do this:
> >
> > pointer = malloc(number);
> > if( number == NULL )

>
> I'm assuming this was supposed to be pointer == NULL ...
>
> > {
> > * * *fprintf(stderr, "An error!\n");
> > * * *exit(EXIT_FAILURE);
> >
> > }
> >
> > Is there a proper way to check the return value of a malloc call? *Are
> > there any benefits to doing it one way over another?

>
> Well generally putting assignments inside other statements is
> considered bad form (exceptions being the 'for' keyword).
>
> I'd still check for NULL eitherway though
>
> if ((pointer = malloc(number)) == NULL) {
> ... error
> }
>
> And not use ! on a pointer (though it's probably fine, I don't care to
> check though).


I don't put assignments inside an "if" because I reckon if I'm doing an
"if" then I'm testing something, and I don't expect it to have
side-effects.

I would also be inclined to say:

pointer = malloc (number);
if (pointer==NULL) return NO_MEMORY;

I view it as the caller's responsibility to decide what to do next.

--
Tim

"That excessive bail ought not to be required, nor excessive fines imposed,
nor cruel and unusual punishments inflicted" -- Bill of Rights 1689
 
Reply With Quote
 
Nick Keighley
Guest
Posts: n/a
 
      05-24-2010
On 24 May, 14:12, Tom St Denis <(E-Mail Removed)> wrote:
> On May 24, 9:03*am, Billy Mays <(E-Mail Removed)> wrote:


> > I typically check my malloc calls like this:

>
> > if( !(pointer = malloc(number)) )
> > {
> > * * *fprintf(stderr, "An error!\n");
> > * * *exit(EXIT_FAILURE);

>
> > }

>
> > ... or something similar. *However, I often see people do this:

>
> > pointer = malloc(number);
> > if( number == NULL )

>
> I'm assuming this was supposed to be pointer == NULL ...
>
> > {
> > * * *fprintf(stderr, "An error!\n");
> > * * *exit(EXIT_FAILURE);

>
> > }

>
> > Is there a proper way to check the return value of a malloc call? *


nope. There are various ok ways. If its clear its ok. Personnally I
use

if ((pointer = malloc (number)) == NULL)
{
fprintf (stderr, "memory error\n");
exit 9EXIT_FAILURE);
}

as I always have to think twice what

if (!pointer)

actually means

> > Are
> > there any benefits to doing it one way over another?


not really. The combined assign and test is a bit shorter. Both are
idiomatic C so shouldn't surprise anyone with more than minimal C
experience.

> Well generally putting assignments inside other statements is
> considered bad form (exceptions being the 'for' keyword).


really? I thought assignment combined with if/while was pretty common.
I use it with fopen, fgets and anything else where you test immediatly
after assigning.

while ((c = fgetc (in_stream)) != EOF)
process_char (c);

> I'd still check for NULL eitherway though
>
> if ((pointer = malloc(number)) == NULL) {
> * ... error
>
> }
>
> And not use ! on a pointer (though it's probably fine, I don't care to
> check though).

 
Reply With Quote
 
Eric Sosman
Guest
Posts: n/a
 
      05-24-2010
On 5/24/2010 9:03 AM, Billy Mays wrote:
> I typically check my malloc calls like this:
>
> if( !(pointer = malloc(number)) )
> {
> fprintf(stderr, "An error!\n");
> exit(EXIT_FAILURE);
> }
>
> ... or something similar. However, I often see people do this:
>
> pointer = malloc(number);
> if( number == NULL )
> {
> fprintf(stderr, "An error!\n");
> exit(EXIT_FAILURE);
> }
>
> Is there a proper way to check the return value of a malloc call? Are
> there any benefits to doing it one way over another?


Either is fine (with the correction noted by Tom St Denis).
The latter form may be more convenient if you're making multiple
requests and would like to check them all at once:

p = malloc(NP * sizeof *p);
q = malloc(NQ * sizeof *q);
r = malloc(NR * sizeof *r);
if (p == NULL || q == NULL || r == NULL) {
free (p);
free (q);
free (r);
complain();
return BAD_NEWS;
}

.... as opposed to

if ((p = malloc(NP * sizeof *p)) == NULL) {
complain();
return BAD_NEWS;
}
if ((q = malloc(NQ * sizeof *q)) == NULL) {
free (p);
complain();
return BAD_NEWS;
}
if ((r = malloc(NR * sizeof *q)) == NULL) {
free (p);
free (q);
complain();
return BAD_NEWS;
}

In the (commoner) case where you're making and checking just
one request, I can't see a reason to prefer either form over the
other. (I do, however, prefer an explicit `== NULL' written out,
as I think it reads more naturally. Chacun son got, though.)

--
Eric Sosman
http://www.velocityreviews.com/forums/(E-Mail Removed)lid
 
Reply With Quote
 
Willem
Guest
Posts: n/a
 
      05-24-2010
Tom St Denis wrote:
) On May 24, 9:03?am, Billy Mays <(E-Mail Removed)> wrote:
)> I typically check my malloc calls like this:
)>
)> if( !(pointer = malloc(number)) )
)> {
)> ? ? ?fprintf(stderr, "An error!\n");
)> ? ? ?exit(EXIT_FAILURE);
)>
)> }
)>
)> ... or something similar. ?However, I often see people do this:
)>
)> pointer = malloc(number);
)> if( number == NULL )
)
) I'm assuming this was supposed to be pointer == NULL ...
)
)> {
)> ? ? ?fprintf(stderr, "An error!\n");
)> ? ? ?exit(EXIT_FAILURE);
)>
)> }
)>
)> Is there a proper way to check the return value of a malloc call? ?Are
)> there any benefits to doing it one way over another?
)
) Well generally putting assignments inside other statements is
) considered bad form (exceptions being the 'for' keyword).
)
) I'd still check for NULL eitherway though
)
) if ((pointer = malloc(number)) == NULL) {
) ... error
) }
)
) And not use ! on a pointer (though it's probably fine, I don't care to
) check though).

Of course it's fine.

I personally like the more natural-language style:


"allocate 'number' bytes into 'pointer', or print an error and exit."

Or, in C:

(pointer = malloc(number)) || (fprintf(stderr, "An error!\n") && exit(1));


The first style proposed by the OP is, in natural-language:

"if can't allocate 'number' bytes into pointer, print an error, then exit."


SaSW, Willem
--
Disclaimer: I am in no way responsible for any of the statements
made in the above text. For all I know I might be
drugged or something..
No I'm not paranoid. You all think I'm paranoid, don't you !
#EOT
 
Reply With Quote
 
Malcolm McLean
Guest
Posts: n/a
 
      05-24-2010
On May 24, 4:03*pm, Billy Mays <(E-Mail Removed)> wrote:
>
> Is there a proper way to check the return value of a malloc call? *Are
> there any benefits to doing it one way over another?
>

I use

ptr = malloc(N * sizeof(int) );
if(!ptr)
goto error_exit;

Usually the expression within the malloc call will be of mid-
complexity, and allocating a wrong size is a frequent source of
errors, so it's best to keep it on a line by itself. Almost always if
a call to malloc fails you'll want to fail the function, so jump to
special cleanup code. This allows you to adapt to changed policy
easily (eg exiting on errors rather then returning -1). Also, quite
commonly, you allocate a fairly complex structure with nested arrays.
It's necessary to destroy the whole thing carefully if it has been
incompletely constructed, so the cleanup code needs to be in one
place.



 
Reply With Quote
 
eps
Guest
Posts: n/a
 
      05-24-2010
Malcolm McLean wrote:
> On May 24, 4:03 pm, Billy Mays <(E-Mail Removed)> wrote:
>> Is there a proper way to check the return value of a malloc call? Are
>> there any benefits to doing it one way over another?
>>

> I use
>
> ptr = malloc(N * sizeof(int) );
> if(!ptr)
> goto error_exit;
>
> Usually the expression within the malloc call will be of mid-
> complexity, and allocating a wrong size is a frequent source of
> errors, so it's best to keep it on a line by itself. Almost always if
> a call to malloc fails you'll want to fail the function, so jump to
> special cleanup code. This allows you to adapt to changed policy
> easily (eg exiting on errors rather then returning -1). Also, quite
> commonly, you allocate a fairly complex structure with nested arrays.
> It's necessary to destroy the whole thing carefully if it has been
> incompletely constructed, so the cleanup code needs to be in one
> place.
>
>
>


obligatory http://xkcd.com/292/
 
Reply With Quote
 
Ersek, Laszlo
Guest
Posts: n/a
 
      05-24-2010
On Mon, 24 May 2010, Willem wrote:

> I personally like the more natural-language style:
>
>
> "allocate 'number' bytes into 'pointer', or print an error and exit."
>
> Or, in C:
>
> (pointer = malloc(number)) || (fprintf(stderr, "An error!\n") && exit(1));


7.20.4.3 "The exit function", p1: "void exit(int status);"

6.5.13 "Logical AND operator", p2: "Each of the operands shall have scalar
type."

6.2.5 "Types", p18: "Integer and floating types are collectively called
arithmetic types", p21: "Arithmetic types and pointer types are
collectively called scalar types".

(pointer = malloc(number))
|| (fprintf(stderr, "An error!\n"), (exit(1), 0));

Cheers,
lacos
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      05-24-2010
Malcolm McLean <(E-Mail Removed)> writes:
> On May 24, 4:03*pm, Billy Mays <(E-Mail Removed)> wrote:
>> Is there a proper way to check the return value of a malloc call? *Are
>> there any benefits to doing it one way over another?
>>

> I use
>
> ptr = malloc(N * sizeof(int) );
> if(!ptr)
> goto error_exit;


Even if ptr is declared as a double*? }

Seriously, the clc-recommended idiom for the malloc call is:
ptr = malloc(N * sizeof *ptr);
or, if you like some extra parentheses;
ptr = malloc(N * sizeof(*ptr));
Do you have some specific reason to repeat the type name?

Particularly since, as you point out:

> Usually the expression within the malloc call will be of mid-
> complexity, and allocating a wrong size is a frequent source of
> errors, so it's best to keep it on a line by itself.


[...]

--
Keith Thompson (The_Other_Keith) (E-Mail Removed) <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
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
How to check whether malloc has allocated memory properly in case ifmalloc(0) can return valid pointer Shivanand Kadwadkar C Programming 83 01-08-2011 08:18 AM
Re: How to check whether malloc has allocated memory properly in caseif malloc(0) can return valid pointer Gene C Programming 0 12-20-2010 05:33 AM
When to check the return value of malloc sandeep C Programming 267 06-18-2010 06:11 PM
When to check the return value of malloc Marty James C Programming 333 02-06-2008 11:10 PM
proper way to return errors to parent process junky_fellow@yahoo.co.in C Programming 2 03-01-2005 06:15 AM



Advertisments