When to check the return value of malloc

[Seebs]

I don't think I've ever seen C code in which a macro is #define'd, then
used, then immediately #undef'ed. I can see the argument in favor of
doing it, but in practice most macros are global anyway.

I've done it very rarely, in cases where I used a macro, say, ten or fifteen
times, because it was initialization code of some sort.

Something like:
#define FOO(x) { x, #x }
struct { int value; char *name; } lookup[] = {
FOO(VAL_MIN),
FOO(VAL_ONE),
FOO(VAL_TWO),
{ 0, 0 }
}
#undef FOO

Basically, "cargo cult programming" means programming by rote
without understanding what you're doing. (The roots of the term
are fascinating, but not really relevant.)

I agree that they're not very relevant, but the image is so evocative I
like to explain it.

A lot of good programmers have been reading your code. Not being able
to understand it isn't the problem.

In a way, though, it is.

There are a couple of questions you must ask about any piece of code you
wish to truly understand:

1. What does it do?
2. What was it intended to do?
3. Why does it need to do that?
4. Why is it doing that in this particular way?

The problem here is not that people can't figure out the answer to question
#1 -- surely, most of us can read this code and see what it's doing. The
problem is that if you write something in an unusual way, it leads the reader
to wonder why you did it that way rather than in a more straightforward
way.

Consider:
void
copystring(char *to, char *from) {
void *t = (void *) to;
void *f = (void *) from;
union { int zero; char c; } u;
u.zero = 0L;
u.c = 0;
do {
memcpy(t, f, 1);
t = (void *) ((char *) t) + 1;
f = (void *) ((char *) f) + 1;
} while (memcmp(f, &u.c, 1));
return;
free(&u);
}

An experienced programmer can probably say that this (unless I screwed it
up, no promises) copies the contents of "from" to "to" up to but not
including the first NUL byte encountered in "from".

But that doesn't lead to *understanding* the code. Why do we use t and f
instead of to and from? Why are we using memcmp to see whether the next
byte is 0x0? Why is u a union? Why is u.zero initialized before u.c, and
why was it initialized at all? Why does the code contain a free of a
non-allocated address, but put this after a return statement so it can't
be executed?

All of these questions suggest that one of two things is at issue:

1. The person who wrote this code had a very primitive, at best,
understanding of pointers in C. The code is probably unreliable.
2. There is something extremely unusual going on which you need to
know about.

In fact, it's really:
3. It is a contrived example.

Sandeep's code reads like a contrived example of some sort -- it acts as
though something special is going on, when nothing can be found to justify
the choices made, and that is a big red flag usually.

-s

 

[Keith Thompson]

Which is another way of saying if a supposedly safe malloc could be
written it would never need an error return.

A safe malloc function could return a pointer to and a size of the
actually allocated space and let the program decide whether to
continue with the smaller allocation or emit an error.

No, that's not what I was saying at all. It didn't even occur to
me that the response to a failure to allocate a certain amount
of memory might be to allocate some smaller amount of memory.
I'm sure that approach makes sense in some cases (for example,
if you're allocating an in-memory file buffer, a smaller buffer is
better than none at all). And I can imagine a highly specialized
allocation function that allocates as much as it can up to what
was requested -- but I wouldn't call such a function a "safe malloc".

For most allocations, if you can't get what you asked for, that's
a failure, and you either need to fall back to some other approach
or prepare to shut down. If I'm trying to allocate a tree node,
half a node does me no good.

 

[Ben Bacarisse]

<snip>

[About pre-allocating memory for a low-memory emergency.]

The entire idea is just plain wrong. You have formed some kind of crazy
theory about "how malloc works", and that theory is incorrect, leading you
to do stuff that makes no sense.

This "crazy theory" might have been formed, in part, by reading the
standard. A straight-forward interpretation of 7.20.3.2 paragraph 2:

"The free function causes the space pointed to by ptr to be
deallocated, that is, made available for further allocation."

suggests that what the OP was doing would work.

You and I may be world-weary enough to know that such wording is never
straight-forward, and implementations are not always conforming, but
perhaps the OP took this (or some paraphrase of it in a textbook) at
face value.

This is like driving a car and making a point of manually triggering the
airbags before you even start the car, so that you'll be safe in the event
of a crash.

Or it's like reading in the car manual "if you trigger the airbags early
they will keep you safe in future crashes" and believing it.

<snip>

 

[Seebs]

This "crazy theory" might have been formed, in part, by reading the
standard. A straight-forward interpretation of 7.20.3.2 paragraph 2:

"The free function causes the space pointed to by ptr to be
deallocated, that is, made available for further allocation."

suggests that what the OP was doing would work.

Ahh, I see. Yes, that would imply that, except of course that "available"
doesn't mean "usable". But I do agree, it might seem that way at first.

Or it's like reading in the car manual "if you trigger the airbags early
they will keep you safe in future crashes" and believing it.

I think possibly reading "The air bags ensure safety in a crash", and
concluding that you should trigger them to be sure you're safe.

-s

 

[Phil Carmody]

I know I was using C++. I doubt C would be any different in this respect.

All that stuff that's just done for you in C++ so you don't have to
worry about may be the stuff that you don't want to happen. Whilst
it's certainly possible, I think you'd have to go a little further
out of your way to get such behaviour in C, you would have to
deliberately design it in more.

However, thanks for sharing your C++ woes on comp.lang.c.

Phil

 

[Phil Carmody]

I think this is wrong. With no "qualifier", the number will be
interpreted as an int and undergo default promotions. Because int may be
16 bits this could overflow.

On the other hand, you could unlearn all the nonsense you've learnt
and actually learn C properly this time. Next time pay more attention
to:
[#5] The type of an integer constant is the first of the
corresponding list in which its value can be represented.

I think this is the same logic but with longer and more complicated
code...

Nope. It's significantly easier to read code. Count the number of
references to the emrgcy variable, for example. You had 6. Seebs
has 4. 2 of yours are demonstrably completely unnecessary, and
are just warts in the code.

I don't know what cargo cult stuff is.

It's stuff from somewhere external that is adopted without proper
understanding of what it is, and what it does, and therefore it is
almost always misused.

I like using advanced C features, yes. It makes programming fun. I think
all good programmers will be able to understand my code.

You are the classic example of "a little knowledge is a dangerous thing".

I pity the maintenance programmer who ever has to maintain your
unreadable illogical code. Which I guess is self-pity - I have
seen plenty of such code written by just-out-of-university or
student workers that was as bad, and been forced to fix it (at
the end of a pointy paycheque, of course, I'd not do that for
fun).

Phil

 

[Phil Carmody]

I partially agree with removing so called "magic numbers" from the code in
favor of more descriptive names where it makes sense to do so.


The logic may be the same; however, Seebs version is much more intuitive
then yours. The extra spacing and indentation provides visual clues that
make it easy to pick up what is going on and is consistant with other
properly indented code. I have seen *many* bugs created when using "?:" to
obfuscate the code that could clearly be seen using the normal if/else
syntax.

The main problem was that he wanted a "?:=" version of gcc's "?:"
operator. If he'd have actually wanted "? :", it wouldn't have been
so bad to use "? :" (I like dense code, I don't like redundant code).

And as an aside, I think ?: should be standardised.

(And please overlook the syntactical sloppiness, I didn't want to have
to explain ?

Phil

 

[Tim Streater]

Seebs wrote:



In the UK, at a STOP sign (black text in red triangle on white
background), you are *required* to use your hand-brake, since you must
bring the vehicle to a complete and safe stop. (But of course you don't
use it /instead/ of the regular brakes - you use the footbrake to stop
the car, and then the handbrake to keep it stopped.)

The difference with the US is that in the UK we only use Stop signs
where there is a junction with some danger to it (such as a steep slope
to the junction, or hedges). So they are quite rare, since with most
junctions a Give Way is quite sufficient.

The US has almost no Give Way signs and lots of Stop signs where they
are not needed. Like, you're going along a road and there is a Stop sign
serving no purpose whatever (not at a junction, not by a school, even).

 

[Richard Bos]

Another way to look at it is that the application decided, by calling
this particular utility function, that it could not recover from
a memory allocation failure. If it could, it should have called
some malloc() or other function that would permit recovery.

In theory, yes. In practice, when I've seen such functions used, either
the code is throwaway or command line utility code, or the programmer
has _assumed_, not decided, that that he could not recover from malloc()
failures. In the latter case, the programmer is almost always wrong. (In
the first ones, the decision is not so much "could not" as "considered
it too much bother", with, for that size program, is more ofter
justifiable.)

Richard

 

[Keith Thompson]

The main problem was that he wanted a "?:=" version of gcc's "?:"
operator. If he'd have actually wanted "? :", it wouldn't have been
so bad to use "? :" (I like dense code, I don't like redundant code).

And as an aside, I think ?: should be standardised.

(And please overlook the syntactical sloppiness, I didn't want to have
to explain ?

Since you're talking about a gcc-specific extension (and since C already
has a ?: operator), perhaps explaining it would have been a good idea.

gcc allows the middle operand of the conditional operator to be
omitted. ``x ? : y'' is equivalent to ``x ? x : y'' except that x
is evaluated only once. It yields the value of x if x is non-zero,
otherwise it yields the value of y.

 

[Keith Thompson]

Phil Carmody wrote:


I construct objects very often in C. What's the problem?

Who said there was a problem?

The word "constructed" is often (but by no means always) C++-specific
jargon. And it turned out the previous poster was using C++.

 

[Seebs]

The difference with the US is that in the UK we only use Stop signs
where there is a junction with some danger to it (such as a steep slope
to the junction, or hedges). So they are quite rare, since with most
junctions a Give Way is quite sufficient.

Ahh! That makes sense. Ours are used any time you're expected to come
to a stop, but there's no expectation of using the hand brake -- just
slowing down enough that you're clearly not-in-motion.

The US has almost no Give Way signs and lots of Stop signs where they
are not needed. Like, you're going along a road and there is a Stop sign
serving no purpose whatever (not at a junction, not by a school, even).

I have never, ever, seen such a thing. I have only seen Stop signs at
intersections of some sort.

-s

 

[Tim Streater]

Ahh! That makes sense. Ours are used any time you're expected to come
to a stop, but there's no expectation of using the hand brake -- just
slowing down enough that you're clearly not-in-motion.


I have never, ever, seen such a thing. I have only seen Stop signs at
intersections of some sort.

Well, this *was* California.

 

[Tim Streater]

Tim Streater wrote:

Actually, they are /so/ rare in the UK that I misreported what they look
like! (They are actually octagonal, red with a thin white border, and
white text.) I don't recall ever coming across one that was there for
spurious reasons.


Well, that's your problem, not mine.

Well, it was when I lived there. Now, of course, it's not.

 

[Seebs]

Well, this *was* California.

Ahh.

Well, then we know what it was there for; safety! Kinetic energy is a
property of matter known to the State of California to increase the risk
of certain kinds of injury*.

-s
[*] In theory, the requirement is that any object which has kinetic energy
shall bear a sticker stating this, and no object which does not have kinetic
energy shall bear such a sticker, as it wouldn't do to alarm people unduly.
Most manufacturers have fine print somewhere in their product kinetic energy
declarations stating that the product shall constitute its own rest frame,
which is why you never see the stickers.

 

[Seebs]

I think you are not seeing the whole picture here. You should think of
alternative situations too.

Maybe you want the allocation to perform a fast sort routine. If you
can't get enough memory, you can just fall back to a slower out-of-core
sorting routine.

Oh, certainly.

But it's still more useful for the larger allocation to Just Fail, so you
can ask for a smaller one.

The point is, in many cases, you *can't* use a smaller allocation, so
automatically giving you a smaller allocation you may not be able to use
is pointless. Better to just succeed or fail, and let the caller decide
whether to try to allocate something else instead.

-s

 

[sandeep]

For most allocations, if you can't get what you asked for, that's a
failure, and you either need to fall back to some other approach or
prepare to shut down. If I'm trying to allocate a tree node, half a
node does me no good.

I think you are not seeing the whole picture here. You should think of
alternative situations too.

Maybe you want the allocation to perform a fast sort routine. If you
can't get enough memory, you can just fall back to a slower out-of-core
sorting routine.

 

[Seebs]

Constant variables in C are not really constants. You couldn't define an
array of size esize for example.

You could, in C99.

But it doesn't matter, because in the context in question (the size of a
block of memory), no constant was needed.

Preprocessor defines are needed - but
making them globally visible is not needed and is harmful when they are
only used once.

Again, your proposed scheme is likely (over time, certain) to result in
multiple places where a value is "used only once" with conflicting values.

Fundamentally...

If it's used only once, and it doesn't need to be the same elsewhere, *it's
not a constant*. The point of a manifest constant is to be a thing which
is in some way always the right value.

You have never demonstrated an actual example of the alleged harm; millions
upon millions of lines of code have been written using values which are
defined before any other code is used, and remain defined throughout the
rest of the program, and which do not cause problems. You have not shown
a single case in which this could cause a problem.

More importantly, if it *did* cause a problem, that would be a GOOD thing!
The only way it could cause a problem would be if you had inconsistent
beliefs about what an alleged constant should be. If that happens, you WANT
it to cause a visible, obvious, failure, at the earliest possible
opportunity, so you can understand the error and fix it.

It really comes across as though you're just trying to be contrary with
anything anyone says about C.

-s

 

[sandeep]

But again, there's no good reason to use a macro rather than a constant
object declaration:

const size_t esize = 0x40000000;

Constant variables in C are not really constants. You couldn't define an
array of size esize for example. Preprocessor defines are needed - but
making them globally visible is not needed and is harmful when they are
only used once.

 

[Keith Thompson]

Constant variables in C are not really constants.

Before posting something like that, please consider the possibility
that I'm already perfectly well aware of it.

The real point is that "const" doesn't mean "constant"; it means
read-only.

You couldn't define an
array of size esize for example.

In this case, you probably couldn't because it's too big. But C99
does allow you to declare arrays with non-constant lengths (VLAs).
But yes, that's a valid concern if your code might be compiled with
a compiler that doesn't support VLAs.

In any case, you didn't use esize as an array length. Declaring it
as a const size_t object would have worked just fine for your code.

Preprocessor defines are needed - but
making them globally visible is not needed and is harmful when they are
only used once.

Yes, preprocessor defines are sometimes needed. Enumeration
constants are often better, but they work only when you know the
values are within the range of type int (an unfortunate limitation
in the language). But this one *wasn't needed*, and your code
would have been clearer if you had defined a const object rather
than a macro.