A solution for the allocation failures problem

[jacob navia]

Very good message Richard. This is one of the best posts
I have read lately here. Comments below

Writing "check for errors and writing response code" for each
call to malloc is, of course, a viable strategy. However there
are issues to be considered. Three such are (a) the
unreliability of "fixup" code, (b) the non-uniformity of error
response, and (c) code littering.

(A) Unreliability: The code in these "failure to allocate"
clauses isn't easy to test properly, and, in the absence of an
allocator wrapper, isn't easy to test at all. This may not
matter if the only action is to write a message to stderr and
call exit, but it definitely is an issue for more elaborate
responses such as request size retries and alternate algorithms.

Exactly. It is not the cost o writing
if (mp == NULL)
but the cost of figuring out for each individual allocation request
what should be done.

In my view, reliable programs (and if we are not interested in
reliable programs why bother to test at all) are developed and
maintained with test harnesses that test the alternatives.

This means that the test harness becomes *impossible* big
because of all the complicated flow of the tests for each
individual malloc request. Consequence: the test harness can't be
written and all that code is not tested at all.

(B) Non-uniformity: Since each "failure to allocate" test clause
is individually written there is no guaranteed uniformity of
response to error conditions.

It depends if you write the code in the morning, you are fresh,
just took your coffee and make a detailed analysis of the function
and where to redirect the flow when an allocation fails, or if you write
it at 17:55 PM and you are in a hurry because you promised to leave
work at 18.00 PM!

[snip]

All of that said, what is one to do? My suggestion is to use a
wrapper for malloc for almost all storage allocation requests.
The "failure to allocate" test and the subsequent error response
(e.g., write an error message and call exit) is in one place
rather than being scattered as multiple copies throughout the
code.

This is a very important point. The scattering of the tests all
around the code produce precisely a lack of control of what to do
in case of malloc failure. It is impossible to change the behavior
of the program in a general manner since all those tests are
not in a single place.

 

[Ioannis Vranos]

A bit more fixed:

>
>
> How can you do this easily under current C?
>
>
> void somefunc(void) try
> {
> int *p= malloc(100*sizeof(*p));
>
> /* ... *.
> }
>
> catch(bad_alloc)
> {

==> static int no_of_failures= 0;

 

[Richard Heathfield]

(e-mail address removed) said:

And so I went to look for best practices, got some C code from
http://www.cpax.org.uk/prg/portable/c/libs/emgen/index.php#download

It handles malloc() failure pretty well: the function which calls
malloc() returns NULL, and its caller happily uses some default
value instead of what it's supposed to do. But it does "handle"
malloc() failure.

That could be done better, I agree. But to crash and burn would be worse,
not better.

Sure, from such a toy program you shouldn't expect much intelligence
(though it could fail instead of saying "all is well"), but we
are talking here "easy to type", aren't we? Or do I miss something,
and it's really not about handling errors but about typing an
"if(mp != NULL)"?

A bonus question: what happens when malloc() fails inside fgetline()
(my hypothesis: nothing, we pretend we hit EOF. But we don't "crash
and burn", which is Good)

Your hypothesis is incorrect; fgetline returns a non-zero error code for an
allocation failure (it happens to be -3, which I really ought to wrap in a
#define). A generic routine such as fgetline can't reasonably decide what
to do on allocation failure, so it has only two options - crash and burn,
or tell the caller. Crash-and-burn would be stupid, so we tell the caller.

 

[Ed Jensen]

This whole thread seems to be based on the fallacy that if(mp != NULL) is
harder to type than if(fp != NULL).

No, it's not.

It's based on the fact that in most applications, dynamic memory is
allocated (and freed) a lot.

 

[Ed Jensen]

char* p0 = malloc(100);
char* p1 = malloc(200);
char* p2 = malloc(300);

What do you do if the second malloc throws?

In my C code, I typically use a pattern something like this:

int ReturnValue = SUCCESS;
char *p0 = NULL;
char *p1 = NULL;
char *p2 = NULL;

if ((p0 = malloc(100)) == NULL)
{
ReturnValue = ENOMEM;
goto ExitFunction;
}

if ((p1 = malloc(200)) == NULL)
{
ReturnValue = ENOMEM;
goto ExitFunction;
}

if ((p2 = malloc(300)) == NULL)
{
ReturnValue = ENOMEM;
goto ExitFunction;
}

/* Do some work with the allocated memory */

ExitFunction:

free(p0);
free(p1);
free(p2);

return ReturnValue;

 

[Richard Heathfield]

Ed Jensen said:

No, it's not.

It's based on the fact that in most applications, dynamic memory is
allocated (and freed) a lot.

In most applications, information comes in via streams and goes out via
streams, a lot (otherwise where are you getting your data and where are
you putting the results?). But nobody is bothering to propose a "solution
for the read/write failures problem" (and rightly so, because there isn't
such a problem). The fact that malloc is called a lot is not an excuse for
calling it badly.

 

[Ian Collins]

More specifically you may apply try-catch to each of the malloc
statements, you can apply try-catch to the whole block of malloc
statements, you can apply try-catch to the entire function body, or you
can apply try-catch inside a caller function.

The first is no different from individual if() blocks, the second two leak.

You do not need to return failure values from called functions to
callers, it is done automatically if you do not catch the exception.

As I said before, it's very hard and therefore error prone to write
exception safe code with the mechanisms C provides for all of the
reasons it's very hard to use multiple points of return. Square that
when exceptions can escape out of functions.

 

[ymuntyan]

(e-mail address removed) said:






That could be done better, I agree. But to crash and burn would be worse,
not better.

Exit with a failure status would be worse?

Your hypothesis is incorrect; fgetline returns a non-zero error code for an
allocation failure (it happens to be -3, which I really ought to wrap in a
#define). A generic routine such as fgetline can't reasonably decide what
to do on allocation failure, so it has only two options - crash and burn,
or tell the caller. Crash-and-burn would be stupid, so we tell the caller.

fgetline() returns an error, yes. What does it caller do?
The caller thinks it hit EOF (that's my hypothesis).

Anyway, the point is: it's not about how it is easy or hard to
type "if (whatever)". It's very easy to type. Here, we have
a program which has that easy to type line. Does it do the
right thing? No. What does it mean? It means that putting
an if() doesn't make your code right.

Oh, and to make sure I'm in your killfile again: fix your code
then talk about incompetence of others.

Yevgen

 

[Ian Collins]

In my C code, I typically use a pattern something like this:

int ReturnValue = SUCCESS;
char *p0 = NULL;
char *p1 = NULL;
char *p2 = NULL;

if ((p0 = malloc(100)) == NULL)
{
ReturnValue = ENOMEM;
goto ExitFunction;
}

But what if you acquire something here...

if ((p1 = malloc(200)) == NULL)
{
ReturnValue = ENOMEM;
goto ExitFunction;
}

and release it here?

 

[Ed Jensen]

In most applications, information comes in via streams and goes out via
streams, a lot (otherwise where are you getting your data and where are
you putting the results?). But nobody is bothering to propose a "solution
for the read/write failures problem" (and rightly so, because there isn't
such a problem). The fact that malloc is called a lot is not an excuse for
calling it badly.

Garbage collection was largely conceived and now enjoys great success
in part due to the fact that many (most?) applications allocate and
free dynamic memory a lot.

i.e., It was recognized as a resource that can (and often should) be
treated specially.

I think you're being intellectually dishonest when trying to compare
it to other resources (such as files). There are similarities, of
course, but it's likely that most applications allocate and free
memory much more often than they open and close files.

 

[Ed Jensen]

Just give them time. It won't be long before someone argues that
checking return values fopen() is impossible to do correctly, so don't
even try.

It's sad to see this thread continue to include so many childish
insults.

 

[Ed Jensen]

Quite the opposite. It's very cost-effective. You just need to
contrast that cost with what it takes to fix, repair and support
hundreds, thousands or millions of customers in the field that are
suddenly experiencing bugs and demanding fixes, and perhaps even
threatening legal action.

Theoretically, the free market will decide which development practices
are most cost effective. Just look at successful software companies
and determine how much peer review they do. (Of course, some software
companies may cheat by illegally leveraging their monopoly, so you'd
have to pick and choose which companies to review very carefully.)

 

[Ed Jensen]

But what if you acquire something here...

and release it here?

I would use the same strategy, except the code following the
ExitFunction label would look a little different. For example:

ExitFunction:

free(p0);
free(p1);
if (fp != NULL) { fclose(fp); }
free(p2);

return ReturnValue;

That is, I'd just let the clean up code determine whether or not the
resource still needed to be cleaned up.

 

[Kenneth Brody]

In my C code, I typically use a pattern something like this:

int ReturnValue = SUCCESS;
char *p0 = NULL;
char *p1 = NULL;
char *p2 = NULL;

if ((p0 = malloc(100)) == NULL)
{
ReturnValue = ENOMEM;
goto ExitFunction;
}

if ((p1 = malloc(200)) == NULL)
{
ReturnValue = ENOMEM;
goto ExitFunction;
}

if ((p2 = malloc(300)) == NULL)
{
ReturnValue = ENOMEM;
goto ExitFunction;
}

/* Do some work with the allocated memory */

ExitFunction:

free(p0);
free(p1);
free(p2);

return ReturnValue;

In situations like this, I typically do something like:

int retval = SUCCESS;
char *p0, *p1, *p2;

p0 = malloc(100);
p1 = malloc(200);
p2 = malloc(300);
if ( p0 == NULL || p1 == NULL || p2 == NULL )
{
retval = FAILURE;
goto ret;
}

/* do stuff */

ret:

free(p0);
free(p1);
free(p2);
return retval;

Of course, this is a simple case where nothing happens between the
mallocs, and failure handling doesn't matter which one failed. But,
that's not entirely rare.

Yes, I do use "goto" here, even though it could be avoided by placing
"do stuff" within an "else" in this case. Sometimes there are other
possible failures within "do stuff" which wouldn't be handled so
cleanly without the "goto".

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:[email protected]>

 

[Kenneth Brody]

jacob navia wrote:
[...]

It depends if you write the code in the morning, you are fresh,
just took your coffee and make a detailed analysis of the function
and where to redirect the flow when an allocation fails, or if you write
it at 17:55 PM and you are in a hurry because you promised to leave
work at 18.00 PM!

[...]

<mode pedant="on" emoticon="">

Wow. My clock only goes to 11:59 PM. Where do you work?

</mode>

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:[email protected]>

 

[Willem]

Ed wrote:
) Theoretically, the free market will decide which development practices
) are most cost effective.

Hiring the cheapest possible developers and pushing out code that
is 'just about good enough' would seem most cost effective, then.


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

 

[Ian Collins]

Ed Jensen wrote:

I would use the same strategy, except the code following the
ExitFunction label would look a little different. For example:

ExitFunction:

free(p0);
free(p1);
if (fp != NULL) { fclose(fp); }
free(p2);

return ReturnValue;

That is, I'd just let the clean up code determine whether or not the
resource still needed to be cleaned up.

Workable, but you can see the complexity of the code increasing along
with the risk of error. The maintenance programmer will have to take
great care when added code between exit points.

 

[Ed Jensen]

Workable, but you can see the complexity of the code increasing along
with the risk of error. The maintenance programmer will have to take
great care when added code between exit points.

I'm open to alternatives.

 

[Richard Heathfield]

(e-mail address removed) said:

fgetline() returns an error, yes. What does it caller do?
The caller thinks it hit EOF (that's my hypothesis).

Oh, I see - fair enough. Yes, the test is against 0, and doesn't handle
malloc failure separately. It ought to. That's a weakness in the program
(not the library), which I ought to remedy. In fact, I was just looking
over the code again, and spotted it's not the only way in which emgen
needs fixing. When I get some time, I'll do something about that.

Oh, and to make sure I'm in your killfile again: fix your code
then talk about incompetence of others.

I don't see why you think a reasonable complaint about my shoddy code would
land you in my killfile. As for "the incompetence of others", incompetence
can be rooted in either (or both) of two problems - ignorance (not having
learned yet) or stupidity (refusing to put into practice the things one
ought to have learned). Ignorance is correctable (or corrigible, perhaps)
and forgivable. Stupidity is harder to correct and less forgivable.
Unfortunately for me, the emgen code shows definite signs of stupidity on
my part - so yes, I ought to go fix that code.

But when I do so, I will fix it to deal with allocation failures properly -
"crash and burn" should be a last resort, not a first response.

 

[jacob navia]

(e-mail address removed) said:


Oh, I see - fair enough. Yes, the test is against 0, and doesn't handle
malloc failure separately. It ought to. That's a weakness in the program
(not the library), which I ought to remedy. In fact, I was just looking
over the code again, and spotted it's not the only way in which emgen
needs fixing. When I get some time, I'll do something about that.


I don't see why you think a reasonable complaint about my shoddy code would
land you in my killfile. As for "the incompetence of others", incompetence
can be rooted in either (or both) of two problems - ignorance (not having
learned yet) or stupidity (refusing to put into practice the things one
ought to have learned). Ignorance is correctable (or corrigible, perhaps)
and forgivable. Stupidity is harder to correct and less forgivable.
Unfortunately for me, the emgen code shows definite signs of stupidity on
my part - so yes, I ought to go fix that code.

But when I do so, I will fix it to deal with allocation failures properly -
"crash and burn" should be a last resort, not a first response.

Well Mr Heathfield, you were treating me of incompetent, and refusing
to accept the possibility that good programmers will always do some
kind of mistake *sometime*.

There you have the proof. The point is, that as we always add code and
code at each malloc call (and there *are* a lot of them), the
probability of making a mistake *increases*.

The solution I proposed in the starting post of this thread is not
"crash and burn" as you put it. The user can define an exit strategy
or recovery measures because a call back is called that the user can
modify at specific places in the code.

This would allow to flexibly modify the strategy when using more
memory for loading more data (no more data can be loaded, not
a fatal error) or when needing some memory to establish a
network connection (that would be a fatal error)

The advantage of that is that we have now a MUCH SMALLER code base to
review and we can concentrate on doing that small part right, instead
of putting the recovery code at each malloc usage, where there
is no possibility in most cases to change the strategy because no
information about the context is readily available.