Why leave the error handling to the caller?

[Chris Dollin]

Of course it's a different kind of problem: it would mean that you've
reached the end of infinity, probably having forgotten to save some
Scotch to find out whether infinite aging makes Scotch infinitely better.

So ... this Turing machine runs on Scotch tape?

Good for sticky problems, then.

 

[Richard Bos]

Of course it's a different kind of problem: it would mean that you've
reached the end of infinity, probably having forgotten to save some
Scotch to find out whether infinite aging makes Scotch infinitely better.

No. It's asymptotic, but to an asymptote that varies with the brand and
variety.

Richard

 

[Christopher Benson-Manica]

So ... this Turing machine runs on Scotch tape?

Good for sticky problems, then.

Careful; puns like that can lead to UB, especially on Mondays.

 

[Malcolm McLean]

No. It's asymptotic, but to an asymptote that varies with the brand and
variety.

My sister has a wonderful saying.

Why value smoothness? Why not value fire?

 

[Malcolm McLean]

Malcolm McLean wrote, On 17/06/07 22:40:

Malcolm McLean wrote:

Malcolm McLean said:

<snip>

There quite a strong case for a safemalloc() library function that
does terminate with an error message on fail.

No, there isn't. Library routines have no business deciding to
terminate
the program.

The problem is that the code is disproprtionate. [...]
[code snipped; see up-thread]

Should fprintf() halt the program on an I/O error?

Should fopen() halt the program if unable to open the file?

Should strtod() halt the program on a malformed input?

Then why should malloc() halt the program on a whim?

The Turing machine has run out of tape. That's a different kind of
problem to the others you have described.

It is a recoverable error in many situations just as all the others are.
For example, when stressing my company notebook several times I've had
VMware tell me it was out of memory giving the option to retry. I closed
down some other stuff, told it to retry, and continued with my work. Far
better than your approach of having it die on me.

But what if you don't have a program at all because with the 33:67 payload
to malloc recovery ratio, it costs too much to write?

Sometimes it's an issue, sometimes not. If you are writing Microsoft's next
OS you have practially infinite resources. If you're knocking up a game on a
tight budget, it is no good missing Christmas. If it crashes out one every
hundred hours of gameplay, that's not ideal but it isn't the end of the
world either. If it is two months late it might be canned, then you get zero
royalties, and no-one will ever play it.

 

[Flash Gordon]

Malcolm McLean wrote, On 19/06/07 20:37:

Malcolm McLean wrote, On 17/06/07 22:40:

Malcolm McLean wrote:

Malcolm McLean said:

<snip>

There quite a strong case for a safemalloc() library function that
does terminate with an error message on fail.

No, there isn't. Library routines have no business deciding to
terminate
the program.

The problem is that the code is disproprtionate. [...]
[code snipped; see up-thread]

Should fprintf() halt the program on an I/O error?

Should fopen() halt the program if unable to open the file?

Should strtod() halt the program on a malformed input?

Then why should malloc() halt the program on a whim?

The Turing machine has run out of tape. That's a different kind of
problem to the others you have described.

It is a recoverable error in many situations just as all the others
are. For example, when stressing my company notebook several times
I've had VMware tell me it was out of memory giving the option to
retry. I closed down some other stuff, told it to retry, and continued
with my work. Far better than your approach of having it die on me.

But what if you don't have a program at all because with the 33:67
payload to malloc recovery ratio, it costs too much to write?

That's rubbish according to your own attempt at showing how bad the
ratio is.

Sometimes it's an issue, sometimes not. If you are writing Microsoft's
next OS you have practially infinite resources.

That's rubbish because the above situation is on a modern (only a couple
of months old) medium to high spec notebook.

On my previous company notebook which was high spec at the time I would
also run out of resources.

I've seen servers run out of resources, including high spec ones.

> If you're knocking up a
game on a tight budget, it is no good missing Christmas. If it crashes
out one every hundred hours of gameplay, that's not ideal but it isn't
the end of the world either. If it is two months late it might be
canned, then you get zero royalties, and no-one will ever play it.

If it crashes too often because it is badly written no one will buy it.
In one extreme case a game got canned because after it was produced they
found it contained a virus, so there they lost a lot more money. I'm
sure others have failed to make it to market, or failed to have
significant sales, due to too many bugs.

 

[CBFalconer]

.... snip ...

But what if you don't have a program at all because with the 33:67
payload to malloc recovery ratio, it costs too much to write?

Sometimes it's an issue, sometimes not. If you are writing
Microsoft's next OS you have practially infinite resources. If
you're knocking up a game on a tight budget, it is no good missing
Christmas. If it crashes out one every hundred hours of gameplay,
that's not ideal but it isn't the end of the world either. If it is
two months late it might be canned, then you get zero royalties,
and no-one will ever play it.

Are you crippled? Write your own:

#include <stdlib.h>
void *xmalloc(size_t sz) {
void *t;

if (!(t = malloc(sz))) exit(EXIT_FAILURE);
return t;
}

Very complex. Will never return NULL.

 

[Malcolm McLean]

Malcolm McLean said:


If you try hard enough, the code /can/ be disproportionate.


I looked at your code, and found it lacking in any significant evidence
that you are a regular clc subscriber.



Get back to me when you've written it properly.


You'd be amazed. Still, look on the bright side - it didn't actually
call gets().


To my mind it was just a bunch of broken code.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct
{
char **str;
int N;
} STRINGS;

STRINGS *makestrings(char *data);
void killstrings(STRINGS *s);

/*
take a comma-separated list of strings, and create
structure with separate strings.
Params: data - the input
Returns: strings object
*/
STRINGS *makestrings(char *data)
{
STRINGS *answer;
char *ptr, *next;
size_t len;
int i;

answer = malloc(sizeof(STRINGS));
if(!answer) /* 1 */
goto error_exit; /* 2 */
answer->str = 0; /* 3 */
answer->N = 1;
for(i=0;data;i++)
if(data == ',')
answer->N++;
if(!answer->N) /* 4 */
return answer; /* 5 */
answer->str = malloc(answer->N * sizeof(char *));
if(!answer->str) /* 6 */
goto error_exit; /* 7 */
for(i=0;i<answer->N;i++) /* 8 */
answer->str = 0; /* 9 */
ptr = data;
for(i=0;i<answer->N;i++)
{
next = strchr(ptr, ',');
if(!next)
next = ptr + strlen(ptr);
len = next - ptr;
answer->str = malloc(len + 1);
if(!answer->str) /* 10 */
goto error_exit; /* 11 */

strncpy(answer->str, ptr, len);
answer->str[len] = 0;
ptr = next + 1;
}
return answer;
error_exit: /* 12 */
killstrings(answer); /* 13 */
return 0; /* 14 */
}

/*
strings object destructor
*/
void killstrings(STRINGS *s)
{
int i;

if(s)
{
if(s->str)
{
for(i=0;i<s->N;i++)
free(s->str);
}
free(s->str);
free(s);
}
}

int main(int argc, char **argv)
{
STRINGS *str;
int i;

if(argc < 2)
{
printf("Call with comma-separated list of strings\n");
exit(EXIT_FAILURE);
}
str = makestrings(argv[1]);
for(i=0;i<str->N;i++)
printf("***%s***\n", str->str);
return 0;
}

Here's a compileable version. I haven't tested for malloc() failures, so not
all the code has actually been exectuted, and it is not easy to do that. My
free Microsoft compiler demanded a reregistration.

For what it is worth I have started a new project - BabyX. It's a
lightweight toolkit built on top of xlib to free me from MS.

 

[Eric Sosman]

CBFalconer wrote On 06/19/07 17:36,:

Malcolm McLean wrote:

... snip ...



Are you crippled? Write your own:

#include <stdlib.h>
void *xmalloc(size_t sz) {
void *t;

if (!(t = malloc(sz))) exit(EXIT_FAILURE);

if (!(t = malloc(sz)) && sz > 0) ...

 

[CBFalconer]

CBFalconer wrote On 06/19/07 17:36,:

if (!(t = malloc(sz)) && sz > 0) ...

Thanks for the correction. I always forget that.

 

[Stephen Sprunk]

CBFalconer wrote On 06/19/07 17:36,:

if (!(t = malloc(sz)) && sz > 0) ...

That cure is almost as bad as the cause, since it violates the
(informal)documentation given.

#include <stdlib.h>
/* like malloc(), but never returns NULL */
void *xmalloc(size_t sz) {
void *t;

t = malloc(sz);

if ((!t) && (!sz))
t = malloc(1);

if (!t) exit(EXIT_FAILURE);

return t;
}

S

 

[Ian Collins]

That cure is almost as bad as the cause, since it violates the
(informal)documentation given.

#include <stdlib.h>
/* like malloc(), but never returns NULL */
void *xmalloc(size_t sz) {
void *t;

t = malloc(sz);

if ((!t) && (!sz))
t = malloc(1);

if (!t) exit(EXIT_FAILURE);

I'd be tempted to replace or precede this line with

assert( t );

to give some indication as to why the program exits unexpectedly.

 

[Flash Gordon]

Malcolm McLean wrote, On 19/06/07 22:39:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct
{
char **str;
int N;
} STRINGS;

STRINGS *makestrings(char *data);
void killstrings(STRINGS *s);

/*
take a comma-separated list of strings, and create
structure with separate strings.
Params: data - the input
Returns: strings object
*/
STRINGS *makestrings(char *data)
{
STRINGS *answer;
char *ptr, *next;
size_t len;
int i;

answer = malloc(sizeof(STRINGS));
if(!answer) /* 1 */
goto error_exit; /* 2 */
answer->str = 0; /* 3 */

This isn't handling a malloc failure, it is is actually completely
pointless the way you have written your code.

answer->N = 1;
for(i=0;data;i++)
if(data == ',')
answer->N++;

Having started answer->N at 1 how do you expect it to reach 0 on
incrementing? Overflowing an int invokes undefined behaviour which is
likely to give you a negative value and screw things up earlier. Of
course, if you had used size_t it would wrap and be easy to trap...

Note that this error trapping would be nothing to do with trapping
malloc failures either.

if(!answer->N) /* 4 */
return answer; /* 5 */
answer->str = malloc(answer->N * sizeof(char *));

<snip>

You really should do some vaguely sane formatting if you want people to
read your code. The above formatting suggests you think the above is in
the for loop which clearly it isn't.

I don't know about other people, but I have little patience with badly
formatted code from someone who has been around long enough to know how
to format sanely, so I've not bothered reading the rest..

 

[Richard Heathfield]

Ian Collins said:

Stephen Sprunk wrote:

I'd be tempted to replace or precede this line with

assert( t );

to give some indication as to why the program exits unexpectedly.

Not in C90 you wouldn't. In C90, you'd be tempted to replace or precede
it with assert(t != NULL);

 

[Richard Heathfield]

CBFalconer said:

#include <stdlib.h>
void *xmalloc(size_t sz) {
void *t;

if (!(t = malloc(sz))) exit(EXIT_FAILURE);
return t;
}

Very complex. Will never return NULL.

That's a great solution for students, but anyone writing code like that
in the Real World needs to be re-trained or, if necessary, transferred
to Marketing.

 

[Richard Heathfield]

Malcolm McLean said:

If it
crashes out one every hundred hours of gameplay, that's not ideal but
it isn't the end of the world either.

True, but it may well be the last game from that company that my kids
ever buy.

If it is two months late it
might be canned, then you get zero royalties, and no-one will ever
play it.

No-one wants to play games written by incompetent programmers, either.

 

[Richard Heathfield]

Flash Gordon said:

I don't know about other people, but I have little patience with badly
formatted code from someone who has been around long enough to know
how to format sanely, so I've not bothered reading the rest..

I took a brief look, but I felt that *either* Malcolm was trying to
write bad code (in which case he was being disingenuous at best) or he
was trying to write good code (in which case I pity his customers).

Hanlon's Razor suggests the latter.

 

[Richard Bos]

My sister has a wonderful saying.

Why value smoothness? Why not value fire?

Feel free to send your sister over to me, and I'll set fire to her leg
hairs.

Richard

 

[CBFalconer]

CBFalconer said:



That's a great solution for students, but anyone writing code like
that in the Real World needs to be re-trained or, if necessary,
transferred to Marketing.

And what do you object to? Kindly elide 'missing braces' and the
possibility of sz == 0.

 

[Dave Vandervies]

CBFalconer said:



That's a great solution for students, but anyone writing code like that
in the Real World needs to be re-trained or, if necessary, transferred
to Marketing.

From the OpenSSH 4.6 source code:
--------
void *
xmalloc(size_t size)
{
void *ptr;

if (size == 0)
fatal("xmalloc: zero size");
ptr = malloc(size);
if (ptr == NULL)
fatal("xmalloc: out of memory (allocating %lu bytes)", (u_long) size);
return ptr;
}
--------
fatal() writes an error message and terminates; this is the only place in
any of the OpenSSH tools (except ssh-keyscan, which isn't typically used
other than for configuration purposes) where malloc is called directly,
and everything else goes through here. Similar wrappers exist for every
other function that dynamically allocates memory.

I wouldn't exactly call SSH "not real world". I think you'd have trouble
arguing that the people who wrote this code didn't carefully consider
what was and wasn't appropriate behavior on memory allocation failure
before they wrote it this way, even if you disagree with their conclusion.


dave