Checking return values for errors, a matter of style?

[goose]

Re: Checking return values for errors, a matter of style?


jacob navia wrote:

Johan Tibell a écrit :

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(socke t, args) == -1) {
perror("functio n");
exit(EXIT_FAILU RE);
}

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("functio n");
exit(EXIT_FAILU RE);
}

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

>
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]

Re: Checking return values for errors, a matter of style?


Johan Tibell wrote:

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(socke t, args) == -1) {
perror("functio n");
exit(EXIT_FAILU RE);
}
>
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("functio n");
exit(EXIT_FAILU RE);
}
>
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 ("translatio n 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;
}


-----------------------
hth,
goose,

[goose]

Re: Checking return values for errors, a matter of style?


Andrew Poelstra wrote:

<snipped>

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]

Re: Checking return values for errors, a matter of style?


Andrew Poelstra wrote:

<snipped>

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,siz e) (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,

[goose]

Re: Checking return values for errors, a matter of style?


Andrew Poelstra wrote:

On 2006-08-01, Richard Heathfield <invalid@invali d.invalidwrote:

Andrew Poelstra said:

It's not so much a matter of style as it is a matter of practibility:
your code must be robust, but it should also be easy to read.

Your code is not going to be robust if it doesn't check whether a request
for an external resource was successful. I agree it should be easy to read,
but that doesn't mean leaving the code out!

>
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.

>

Nowadays
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.

Programmers who make such an assumption should not be writing for the home
market or the server market. They should be writing in crayon on droolproof
paper.

>
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.
<otThats 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,

[Flash Gordon]

Re: Checking return values for errors, a matter of style?

Keith Thompson wrote:

rlb@hoekstra-uitgeverij.nl (Richard Bos) writes:

>Keith Thompson <kst-u@mib.orgwrote:

[...]

>>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.

>
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.
--
Flash Gordon
Still sigless on this computer.

[Keith Thompson]

Re: Checking return values for errors, a matter of style?

"goose" <ruse@webmail.c o.zawrites:

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 (The_Other_Keit h) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Keith Thompson]

Re: Checking return values for errors, a matter of style?

"goose" <ruse@webmail.c o.zawrites:

Andrew Poelstra wrote:
<snipped>
>

>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,siz e) (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(s ize) ? (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_FAILU RE).

--
Keith Thompson (The_Other_Keit h) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Richard Heathfield]

Re: Dealing with memory allocation failure

Skarmander said:

<snip>

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>


--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999

http://www.cpax.org.uk

email: rjh at above domain (but drop the www, obviously)

[goose]

Re: Checking return values for errors, a matter of style?


Keith Thompson wrote:

<snipped>

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]

Re: Checking return values for errors, a matter of style?


Keith Thompson wrote:

"goose" <ruse@webmail.c o.zawrites:

<snipped>

#define FMALLOC(ptr,siz e) (ptr=malloc (size) ? ptr : exit (-1))

<snipped>

>
I'd write that as:
>
#define FMALLOC(ptr, size) ((ptr)=malloc(s ize) ? (ptr) : exit (EXIT_FAILURE))
>

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

#define FMALLOC(ptr,siz e) ((ptr)=malloc(s ize) ? (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]

Re: Checking return values for errors, a matter of style?

On Wed, 2 Aug 2006, goose wrote:

>
Keith Thompson wrote:

>"goose" <ruse@webmail.c o.zawrites:

>
<snipped>
>

>>>
>>#define FMALLOC(ptr,siz e) (ptr=malloc (size) ? ptr : exit (-1))

<snipped>

>>
>I'd write that as:
>>
>#define FMALLOC(ptr, size) ((ptr)=malloc(s ize) ? (ptr) : exit (EXIT_FAILURE))
>>

>
EXIT_FAILURE really *is* what I should've used. However I'll change
the above to:
>
#define FMALLOC(ptr,siz e) ((ptr)=malloc(s ize) ? (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]

Re: Checking return values for errors, a matter of style?

"goose" <ruse@webmail.c o.zawrites:

Keith Thompson wrote:

>"goose" <ruse@webmail.c o.zawrites:

>
<snipped>
>

>
#define FMALLOC(ptr,siz e) (ptr=malloc (size) ? ptr : exit (-1))

<snipped>

>>
>I'd write that as:
>>
>#define FMALLOC(ptr, size) ((ptr)=malloc(s ize) ? (ptr) : exit (EXIT_FAILURE))

>
EXIT_FAILURE really *is* what I should've used. However I'll change
the above to:
>
#define FMALLOC(ptr,siz e) ((ptr)=malloc(s ize) ? (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".

--
Keith Thompson (The_Other_Keit h) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[goose]

Re: Checking return values for errors, a matter of style?

Keith Thompson wrote:

"goose" <ruse@webmail.c o.zawrites:

<snipped>

(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".
>

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

Keith Thompson (The_Other_Keit h) kst-u@mib.org <http://www.ghoti.net/~kst>

<grinshouldn' t that be "The_Other_Thom pson" ?

goose,

[Morris Dovey]

Re: Checking return values for errors, a matter of style?

Johan Tibell (in
1154450851.7496 38.215370@b28g2 00...legr oups.com) 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.

--
Morris Dovey
DeSoto Solar
DeSoto, Iowa USA

http://www.iedu.com/DeSoto