Checking return values for errors, a matter of style?

[Johan Tibell]

It is now.

Then quoting my question in your post as if your responding to some
claim I made makes no sense at all.

 

[jacob navia]

Sjouke Burry a écrit :

The only time I used a debugger in 35 years of
programming, was when an assembler decoding
subroutine failed to cross a 64K barrier properly.
(I did not write that one.).
To me, it smells like "let the system catch my errors"
while you should try to avoid them in the first place.
Sloppy typing can cause errors which are not
found by your compiler/debugger.

Well, is a question of taste. I am not a wizard/genius
programmer and I think I will never find all possible
situations that my software will find. Specially when
interacting with other party software that is underspecified
(like in the case of writing a linker, or an assembler)
there is no other way to know "what is that routine returning"
actually.

And yes, debuggers speed up the development, specially if
used within a good IDE. "Go to definition" "Show usage of"
and many other tools that modern IDEs offer are now a
second nature to me.

Yes, sometimes you do not have a debugger, and then
my first task is usually to write one. Last embedded system
I did was a DSP with 16 bit CPU and 80K RAM. The first thing
I wrote for it was a debugger that used the serial port.

You write all programs correctly in 35 years... never need
a debugger... OK. That's YOUR way of doing things.

In 35 years of coding I never wrote a significant system without having
to go in the debugger to develop it.

Obviously "hello world" type of programs run the first time. But
serious systems?

jacob

 

[Richard Bos]

The response to a failure depends on the situation. I've covered this in
some detail in my one-and-only contribution to "the literature", so I'll
just bullet-point some possible responses here:

* abort the program. The "student solution" - suitable only for high school
students and, perhaps, example programs (with a big red warning flag).
* break down the memory requirement into two or more sub-blocks.
* use less memory!
* point to a fixed-length buffer instead (and remember not to free it!)
* allocate an emergency reserve at the beginning of the program
* use virtual memory on another machine networked to this one (this
is a lot of work, but it may be worth it on super-huge projects)

[...]

Out of curiosity, how often do real-world programs really do something
fancy in response to a malloc() failure?

Depends on the situation. For example, if the program is a word
processor, then I would hope that the response to a malloc() failure
that occurs when trying to paste a humungous graphic would be to put up
a message saying "sorry, no memory to paste this picture" and keep the
rest of the document in a workable, savable condition, and not to throw
up your hands and die, trashing the still usable existing document.

I've seen suggestions that, if a malloc() call fails, the program can
fall back to an alternative algorithm that uses less memory. How
realistic is this? If there's an algorithm that uses less memory, why
not use it in the first place? (The obvious answer: because it's
slower.) Do programmers really go to the effort of implementing two
separate algorithms, one of which will be used only on a memory
failure (and will therefore not be tested as thoroughly as the primary
algorithm)?

I would expect that for, say, sorting code this is not all that unusual.
Ditto for, say, handling massive queries in database servers.

Richard

 

[Keith Thompson]

Depends on the situation. For example, if the program is a word
processor, then I would hope that the response to a malloc() failure
that occurs when trying to paste a humungous graphic would be to put up
a message saying "sorry, no memory to paste this picture" and keep the
rest of the document in a workable, savable condition, and not to throw
up your hands and die, trashing the still usable existing document.

Thank you, that's an excellent example.

In general, a batch computational program will perform a series of
operations, and if any of them fails it's likely (but by no means
certain) that the best you can do is throw out the whole thing. An
interactive program, on the other hand, performs a series of tasks
that don't necessarily depend on each other, so it makes more sense to
abort one of them and continue with the others.

 

[Skarmander]

Skarmander said:


<grin> No, there's no harumphing going on over here. But basic resource
acquisition checking is as fundamental to robustness as steering so as not
to bump the kerb is to driving.

Checking, yes. Failure strategies are another matter.

A program that merrily malloc()s along and does not check the return value
is brittle, no argument there. A program that checks and decides to quit at
that point need not be -- but see below.

Absolutely. And a dead customer is a lost customer, right?

Worse, a dead customer is a lost customer whose relatives will probably
claim. But let us not "bicker and argue over who killed who"...

Or not even that. We're back to undefined behaviour.

That's not what I meant. A program that deals with memory exhaustion by
exiting cleanly may actually offer enough guarantees to the customer, as far
as the customer is concerned. If that's the case, we needn't worry about
dead customers (as the customer has done this for us), and we'd probably
waste time doing so.

If the objective is to gather enough moisture to survive until the rescue
helicopter arrives - that is, if the objective is to complete the immediate
task so that a consistent set of user data can be saved before you bomb out
- then it's a sensible approach.

Yes, this (make sure you're in a consistent state before bailing out) is the
sense in which robustness applies to memory exhaustion.

Your initial post was confusing because you made it look as if exiting with
an error was unacceptable, as the "high school solution", while what you
meant (I take it) was that a program should respond by working towards a
consistent state, and then (as it eventually will have to) bail out. ("Wait
for more memory to become available" may be an approach for a very limited
set of circumstances, but it's not recommendable in general at all, as it'll
probably lead to deadlocks.)

If the high school student has looked at the program carefully and decided
that, at the point of memory exhaustion, the program state was consistent
enough (and this is given by the specifications, explicit or not), then
simply exiting is altogether acceptable. My argument was that many "high
school exits" are acceptable in just that way. Not all, and exiting as a
blanket approach to everything isn't acceptable, but in many cases that lone
exit() is just what is called for.

Your other recommendations were useful strategies to trade some (often
negligible) performance for more social memory requirements. I see how in
many cases these might be considered as making things more robust in a given
environment, however, so it's mostly arguing semantics.

I was assuming just the one, actually, local to the function where the
storage is needed.

My bad, I was thinking in terms of a global approach to memory allocation
(wrappers around malloc() and the like).

S.

 

[goose]

Johan Tibell a écrit :

Using the second form allows you to easily see the return value in
the debugger.

Unless the enthusiastic compiler optimised the variable
away (its not really needed in the above example, unless
it gets tested again).

goose,

 

[goose]

I've written a piece of code that uses sockets a lot (I know that
sockets aren't portable C, this is not a question about sockets per
se). Much of my code ended up looking like this:

if (function(socket, args) == -1) {
perror("function");
exit(EXIT_FAILURE);
}

I feel that the ifs destroy the readability of my code. Would it be
better to declare an int variable (say succ) and use the following
structure?

int succ;

succ = function(socket, args);
if (succ == -1) {
perror("function");
exit(EXIT_FAILURE);
}

What's considered "best practice" (feel free to substitute with: "what
do most good programmers use")?

You're in luck , here is a slightly modified (incomplete) function
that I wrote (during the course of play in the last 30 minutes.

The macros ERROR and DIAGNOSTIC are (currently) identical
and merely print the message to screen (with filename, line
number and function name):

----------------------
#define TEST_INPUT ("test.in")
#define TOK_FOPEN (1)
#define TOK_FERROR (2)
#define TOK_INIT (3)
bool test_token (void)
{
FILE *in = fopen (TEST_INPUT, "r");
jmp_buf handler;
int e, c, counter;

/* All errors caught and handled here */
if ((e = setjmp (handler))!=0) {
switch (e) {
case TOK_FERROR:
ERROR ("read error on '%s', read %i bytes\n",
TEST_INPUT, counter);
break;

case TOK_INIT:
ERROR ("unable to initialise '%s' for reading\n",
TEST_INPUT);
break;

case TOK_FOPEN:
ERROR ("unable to open file '%s' for reading\n",
TEST_INPUT);
break;

default:
ERROR ("unknown error\n");
break;
}
if (in) {
fclose (in); in = NULL;
}
return false;
}


/* Meat of function */
if (!in) longjmp (handler, TOK_FOPEN);

DIAGNOSTIC ("translation of '%s' started\n", TEST_INPUT);

c = fgetc (in);
counter = 0;

if (!token_init ()) longjmp (handler, TOK_INIT);

while (c!=EOF) {
counter++;
if (feed_char (c)) {
/* new token awaits us */
}
c = fgetc (in);
}
if (ferror (in)) {
longjmp (handler, TOK_FERROR);
}
printf ("\n");
return true;
}

 

[goose]

Andrew Poelstra wrote:

programs targeted at home computers or servers can assume that you'll
have a 99.99% success rate on a functioning system when allocating
memory < 1Kb.

Statistically, a 99.99% success rate means that your
program will *certainly* fail in the future.

goose,
Smile, its a joke

 

[goose]

Andrew Poelstra wrote:

Because that tiny percentage is the difference between
p = malloc (sizeof *p * q);

and
p = malloc (sizeof *p * q);
if (rv = !!p)
{
/* Rest of function here. */
}

Those ifs nest up and it becomes a pain to manage them.

Then don't; when your number[1] is up and the malloc(10)
call fails, rather exit immediately than have the program
behave unpredictably.

#define FMALLOC(ptr,size) (ptr=malloc (size) ? ptr : exit (-1))
....
char *p; FMALLOC(ptr, sizeof *p * q)
....
Tracking UB is a bloody nightmare for the maintainer!!!
Being unable to reproduce the bug is morale-killer.

[1] When your 0.02% or whatever finally comes up.

goose,

 

[Flash Gordon]

Thank you, that's an excellent example.

In general, a batch computational program will perform a series of
operations, and if any of them fails it's likely (but by no means
certain) that the best you can do is throw out the whole thing. An
interactive program, on the other hand, performs a series of tasks
that don't necessarily depend on each other, so it makes more sense to
abort one of them and continue with the others.

Also when you have an application with a footprint of a few meg and only
a hundred users on a dedicated server with a minimum of 2GB of RAM it is
not worth worrying about doing anything beyond cleanly exiting the
application if malloc fails. Certainly in the last 6 years (since I
joined the company) I've never heard of the application failing due to a
malloc failure, so the work to put in more sophisticated memory handling
would be waisted. Our time is better spent in *this* situation on things
the customer will see such as supporting new government regulations on
the handling of sub-contractors.

 

[goose]

What exactly /would/ be the way to do such a thing? I ask you because
you don't like multiple returns or the break statement, both of which
would be a typical response.

See my setjmp/longjmp "solution" above; yes its dirty but it
removes the error recovery code from the logic so that the logic
at least can look clean. It also lets you go mad with error recovery
without you having your logic all messed up.

Being as every other post was pretty much exactly as insulting as this,

have you met Dan Pop yet ?

I'd say that I wasn't not wrong on any minor point! I'm glad that I haven't
had the chance to make these foolhardy changes to my actual code yet.

s/not//
This /point/ applies to any resource, not just memory.
<ot> Thats what I battle to get into most programmers
skulls: GC doesn't really help as memory is just another
resource and should be treated as such i.e. the language
(and/or compiler) may let you forget all about managing
memory, but you'll still have to do it resource management
anyway. So don't get too smug about your GC language.
All that will happen is that you'll lose the force-of-habit
action that comes with using malloc and forget to free some
*other* resource.

I've written a new interface to my error library so that it will be able
to handle memory failures gracefully, log to a runtime-determined file,
check for bad files or memory, and ensure that a proper message reaches
the user if it can't go on.

Good for you

goose,

 

[Keith Thompson]

jacob navia wrote: [snip]

Using the second form allows you to easily see the return value in
the debugger.

Unless the enthusiastic compiler optimised the variable
away (its not really needed in the above example, unless
it gets tested again).

<OT>
Compilers typically have an option to enable debugging (by retaining
symbol information in object files and executables), and other options
to enable optimizations. Sometimes these options can't be used
together. It's possible to debug optimized code, but it can be
difficult (more for the implementer than for the user).
</OT>

 

[Keith Thompson]

Andrew Poelstra wrote:

Because that tiny percentage is the difference between
p = malloc (sizeof *p * q);

and
p = malloc (sizeof *p * q);
if (rv = !!p)
{
/* Rest of function here. */
}

Those ifs nest up and it becomes a pain to manage them.

Then don't; when your number[1] is up and the malloc(10)
call fails, rather exit immediately than have the program
behave unpredictably.

#define FMALLOC(ptr,size) (ptr=malloc (size) ? ptr : exit (-1))
...
char *p; FMALLOC(ptr, sizeof *p * q)
...
Tracking UB is a bloody nightmare for the maintainer!!!
Being unable to reproduce the bug is morale-killer.

[1] When your 0.02% or whatever finally comes up.

I'd write that as:

#define FMALLOC(ptr, size) ((ptr)=malloc(size) ? (ptr) : exit (EXIT_FAILURE))

I made two changes: I parenthesized the reference to ptr (it's easier
to add parentheses than to figure out when they're not necessary), and
I changed the non-portable exit(-1) to exit(EXIT_FAILURE).

 

[Richard Heathfield]

Skarmander said:

Yes, this (make sure you're in a consistent state before bailing out) is
the sense in which robustness applies to memory exhaustion.

Your initial post was confusing because you made it look as if exiting
with an error was unacceptable, as the "high school solution", while what
you meant (I take it) was that a program should respond by working towards
a consistent state, and then (as it eventually will have to) bail out.

Surprisingly often, that "consistent state" turns out to be achievable only
by running to completion. At least, that has been my experience.

("Wait for more memory to become available" may be an approach for a very
limited set of circumstances, but it's not recommendable in general at
all, as it'll probably lead to deadlocks.)

Well, you can shove in some more RAM chips, right? Or you can shut down a
bunch of random junk that you didn't really need as much as you need this.
I agree there's no point in a /program/ waiting for more memory.

<snip>

 

[goose]

Keith Thompson wrote:

I've seen suggestions that, if a malloc() call fails, the program can
fall back to an alternative algorithm that uses less memory. How
realistic is this? If there's an algorithm that uses less memory, why
not use it in the first place? (The obvious answer: because it's
slower.) Do programmers really go to the effort of implementing two
separate algorithms, one of which will be used only on a memory
failure (and will therefore not be tested as thoroughly as the primary
algorithm)?

<anecdote>

Yes; I've done this once, about three years ago. The task
was to merge 3 sorted lists (original list, records to
remove, records to add) with the resultant list being
around 32000 records long.

The /quick/ way was to merely copy everything into a new
list and add (or don't copy) the records that were to be
added (or removed, respectively) which took about
30 seconds. This worked fine when the developer tested
it, and the software was subsequently shipped on a few K
devices.

Sadly, the device that the developer had on *her* desk
was the "new range" of the devices which had a full
4 megs of memory. The devices *shipped* were our
older stock and had only 2 megs of memory.

Hilarious, I know :-(

After I came up with an algorithm that did all the sorting
and merging in place, we found that the customer
*didn't* want it (because the /slow/ algorithm took almost
10 minutes due to a stupid bus design made by the
stupid hardware folk who assumed that we will
always access memory at least a few K at a time)
but they *also* didn't want their (now broken[1])
devices to sit in the field gathering dust.

The compromise was to make as many devices
as possible (we had a mechanism for reloading
software in the field) use the /fast/ algorithm
and *only* use the slow one if no memory was
available. The customer (in this case one of the
biggest banks on the african continent, AFAIK)
was satisfied with that. There's currently between
19k and 30k of that older device in the field,
all using the slow algorithm until the customer
gets around to buying newer units from us to
replace them.



[1] Because, in addition to very stupidly testing
with a newer device, this genius *also*
never checked the return status of *any* function
call, including all the memory ones. However, she
seemed very well-connected (politically) within the
company and I was *still* her junior when she left
a year ago.

Her replacement is slightly better, but
it really does gall me that I, the junior, *still* have
to fix the code of the senior who says "The C standard
doesn't apply to us, int is 16 bits and i = i++ works well
if you know what you are doing".


I guess the moral of the story is: *always* assume
that the return value is going to say "Wotcha!"
or something similar and be prepared to take
steps, even if the only steps you take are to
print message and die.


goose,
Ok, so I'm a little bitter

 

[goose]

I'd write that as:

#define FMALLOC(ptr, size) ((ptr)=malloc(size) ? (ptr) : exit (EXIT_FAILURE))

EXIT_FAILURE really *is* what I should've used. However I'll change
the above to:

#define FMALLOC(ptr,size) ((ptr)=malloc(size) ? (ptr) : exit
(EXIT_FAILURE))

(you may not leave a space between ptr and size, the preprocessor
stops parsing at the first whitespace and uses the text it found till
the
first whitespace as the macro)

goose,

 

[Tak-Shing Chan]

EXIT_FAILURE really *is* what I should've used. However I'll change
the above to:

#define FMALLOC(ptr,size) ((ptr)=malloc(size) ? (ptr) : exit
(EXIT_FAILURE))

(you may not leave a space between ptr and size, the preprocessor
stops parsing at the first whitespace and uses the text it found till
the
first whitespace as the macro)

Completely wrong. Please turn to pages 154--157 of ISO/IEC
9899:1999 (E) for numerous counterexamples.

Tak-Shing

 

[Keith Thompson]

EXIT_FAILURE really *is* what I should've used. However I'll change
the above to:

#define FMALLOC(ptr,size) ((ptr)=malloc(size) ? (ptr) : exit
(EXIT_FAILURE))

(you may not leave a space between ptr and size, the preprocessor
stops parsing at the first whitespace and uses the text it found
till the first whitespace as the macro)

Nope.

The lack of whitespace between FMALLOC and the '(' is significant
(with whitespace, it would be a macro taking no arguments, and the '('
would be the first token of the expansion). But there's no rule about
whitespace between "ptr," and "size".

 

[goose]

Nope.

The lack of whitespace between FMALLOC and the '(' is significant
(with whitespace, it would be a macro taking no arguments, and the '('
would be the first token of the expansion). But there's no rule about
whitespace between "ptr," and "size".

Thanks (to Tak-Shing too); you live, you learn

Keith Thompson (The_Other_Keith) (e-mail address removed) <http://www.ghoti.net/~kst>

<grin> shouldn't that be "The_Other_Thompson" ?

goose,

 

[Morris Dovey]

Johan Tibell (in
(e-mail address removed)) said:

| Morris Dovey wrote:
|| IMO, "best practice" is to detect all detectable errors and recover
|| from all recoverable errors - and to provide a clear explanation
|| (however terse) of non-recoverable errors.
||
|| I have difficulty imagining that any "good programmer" would ignore
|| errors and/or fail to provide recovery from recoverable errors in
|| production software.
|
| The topic of this post is not whether to check errors or not. Read
| the original message.

Strangely enough, I did exactly that. I responded to the question
asked after looking at the two code snippets and considering both
snippets in terms of "best practice".

I don't find either style difficult to read - but I'll admit that I'm
generally not keen on adding variables and/or bloating the executable
in order to make the source prettier.

YMMV.