Proper way to check malloc return
 

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

Re: Proper way to check malloc return
 

On May 24, 9:03*am, Billy Mays <no...@nohow.com> 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

Re: Proper way to check malloc return
 

In article
<93814643-325d-4a76-b25f-8fa15df6a9dd@u7g2000vbq.googlegroups.com>,
Tom St Denis <tom@iahu.ca> wrote:

> On May 24, 9:03*am, Billy Mays <no...@nohow.com> 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

Re: Proper way to check malloc return
 

On 24 May, 14:12, Tom St Denis <t...@iahu.ca> wrote:
> On May 24, 9:03*am, Billy Mays <no...@nohow.com> 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).

Re: Proper way to check malloc return
 

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 goût, though.)

--
Eric Sosman
esosman@ieee-dot-org.invalid

Re: Proper way to check malloc return
 

Tom St Denis wrote:
) On May 24, 9:03?am, Billy Mays <no...@nohow.com> 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

Re: Proper way to check malloc return
 

On May 24, 4:03*pm, Billy Mays <no...@nohow.com> 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.

Re: Proper way to check malloc return
 

Malcolm McLean wrote:
> On May 24, 4:03 pm, Billy Mays <no...@nohow.com> 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/

Re: Proper way to check malloc return
 

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

Re: Proper way to check malloc return
 

Malcolm McLean <malcolm.mclean5@btinternet.com> writes:
> On May 24, 4:03Â*pm, Billy Mays <no...@nohow.com> 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*? 8-)}

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) kst-u@mib.org <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"