slightly OT: error handling conventions

R

Richard Heathfield

Sheldon said:
Sorry, I'm not Ian, but I would like to ask a question of my own:

What would be the advantage of eliminating goto in a situation
like this?

When reading code, I find goto irritating, because it makes me search
through the code looking for a label. Since I read my own code quite a lot,
I tend to avoid using goto myself. (To say "tend to avoid" is perhaps an
understatement, since I haven't used a goto in real C code in all the years
I've been using the language. I was a big GOTO fan in BASIC, but I
eventually discovered that my BASIC programs, whilst quite cool to my
unjaundiced eye, were largely write-only.)
I once got involved in a large, er, "discussion" on the German C
group about goto. No matter what situation was presented, the
majority of those participating agreed that the use of goto was
an abomination that should be avoided at all costs. Many of the
same people also claimed to refuse to use break and continue.

I wouldn't say "at all costs", but I would avoid goto if there were a more
readable choice. So far, there always has been, at least for me. As for
break, I use it only in switches. Whilst there is a case to be made for
continue in dismissing early exceptions to a loop - for example:

if(line[0] == '#') /* comment - skip it */
{
continue;
}

nevertheless I tend to use continue only to self-document empty loops in an
uncompromising manner (the lack of loop contents could be said to be
self-documenting for an empty loop, but it could also be read as
"programmer forgot loop contents"!).
Their reasoning for this was hard for me to understand, since I
lacked the cultural background, but apparently some influential
professor had some ideas about program design which led most of
them to require all programs to be designed by drawing boxes
inside of boxes. Control was only permitted to flow into the
top of a box or out of the bottom of a box.

That restriction makes code much easier to visualise, IMHO.
This was all nice, and I understood it to be a graphical approach
to structured programming, but what I didn't understand was the
attitude that *absolutely no* deviation from this schema was to
be tolerated.

No, I don't understand that either. The overriding priority is, of course,
correctness. After that, though, I'd go for readability. I'm ready to drop
my structural ascetism tomorrow, or even today, if I happen to find an
alternative that I find more readable. And that alternative, for all I
know, might well include goto.
 
R

Richard Heathfield

CBFalconer said:
You have a hidden assumption here, i.e. that failure does not
malloc(&b).

I was guided entirely by the function name, which I took to be
self-documenting, as all good function names are. :)
 
E

E. Robert Tisdale

Jack said:
I may get blasted for this

You are so correct.
but I've always liked this one:

int my_func()
{
char *a = NULL;
char *b = NULL;
FILE *fin = NULL;
FILE *fout = NULL;
int x = SUCCESS:

do
{
a = malloc(...);
b = malloc(...);
if (!a || !b)
{
x = OUT_OF_MEMORY;
break;
}

fin = fopen(...);
fout = fopen(...);
if (!fin || !fout)
{
x = FILE_OPEN_ERROR;
break;
}

/* do the real work */
} while (0);

free(a);
free(b);
if (fin) fclose(fin);
if (fout) fclose(fout);
return x;
}

This is a good example of *very* poor programming practice.
Of course the do { ... } while (0) with breaks is just a disguised
goto, as indeed are all flow control structures, but inside a block
like this you know they all go to the same place, you don't have to
mentally check the goto labels to see if they are all the same.

As an aside, I have often wished that fclose(NULL) was defined as a
no-op, as is free(NULL), for the same reason of symmetry. Then you
could always fclose() the pointer returned by fopen() whether
successful or not, just as you can always free() the pointer returned
by *alloc(), successful or not.

int my_func(void) { // returns NO_ERROR or ERROR
int error = N0_ERROR; // default return value
char* a = (char*)malloc(some_number);
if (NULL != a) {
int x = another_function(a);
if (ERROR_CONDITION == x) {
error = ERROR;
}
else { // ERROR_CONDITION != x
char* b = NULL; // define variables & constants
// close to first use.
int x = func_that_mallocs(&b);
if (ERROR_CONDITION == x) {
error = ERROR;
}
free(b); // free in same scope as malloc
}
}
free(a); // free in same scope as malloc
return error; // single exit point
}

1. C functions should have a single exit point.
2. Don't declare undefined variables.
3. Place veariable and constant definitions
as close to first use as possible.
4. Don't declare variables in an enclosing scope
just to "reuse" the memory allocated for them.
5. Free allocated memory as soon as possible.
 
M

Mark Gordon

For code where I have multiple resources to find, use, then release I
often use the idea of a 'runlevel'. Basically, it's this:

int do_stuff(char * iname, char * oname) {
int rl=0; /* the 'runlevel' variable */
int rv=ERROR; /* the error return value */
FILE * inf;
FILE * outf;
void * mem;

inf=fopen(iname,"r");
if (!inf) goto on_err;
else rl++; /* opened a resource, go up a level */

outf=fopen(oname,"w");
if (!outf) goto on_err;
else rl++; /* opened a resource, go up a level */

mem=malloc(WHATEVER_SIZE);
if (!mem) goto on_err;
else rl++; /* opened a resource, go up a level */

/* do stuff with inf, outf and mem */

rv=0; /* the success return value */

on_err:
switch(rl) {
case 3: free(mem);
case 2: fclose(outf);
case 1: fclose(inf);
case 0: /* nothing to clean up */
}

return rv;
}

There are plenty of ways this can be improved, but I find this
particular method to be easy to code and easy to analyse. I wouldn't
expect most people to just 'see' what was going on, so it's likely to
be a pain for someone else to maintain - at least until they're
familiar with the idea.

I've used a switch for doing cleanup like this as well, although I used
the setting of an error code on failure to control the clean up. In my
case cleanup also included sending messages over a comms link.

You could get rid of the gotos by using a loop and break to escape on
error.
 
J

Jirka Klaue

E. Robert Tisdale wrote:
....
This is a good example of *very* poor programming practice. ....
1. C functions should have a single exit point.

if ((f = fopen(fn, "r")) == 0) {
fprintf(stderr, "error opening %s\n", fn);
return 0;
}

Are you suggesting that the return statement
should be replaced by a goto? ;-)

Jirka
 
E

Ed Morton

Where is the brace?

They were out of them at the store. Apparently it's the end of the season and I
just missed the clearance sale.

In many cases, it is relatively easy to determine what needs to be
undone without explicitly keeping track of it. This leads to the much
simpler:

void foo()
{
if (a() != OK) goto cleanup;
if (b() != OK) goto cleanup;
c();

cleanup:
if (donec) undoc();
if (doneb) undob();
if (donea) undoa();
}

Note that this is pseudocode, the done* things being expressions, rather
than variables and the undo* things being cleanup actions, rather than
actual function calls.

OK, then how would you approach the OPs specific function? I'd expect your
answer to look like this:

int my_func() {
char *a, *b;
int x, ret = ERROR;

a = malloc(some_number);
if (a == NULL) goto cleanup;

x = another_function(a);
if (x == ERROR_CONDITION) goto cleanup;

x = func_that_mallocs(&b);
if (x == ERROR_CONDITION) goto cleanup;

ret = NO_ERROR;

cleanup:
free(b);
free(a);

return ret;
}

but this in theory invokes undefined behavior if func_that_mallocs() is written
as (pseudo-code):

int func_that_mallocs(char **b) {
*b = malloc(...);
.... /* ensure b is not NULL and do other things */
if (today_is_tuesday) {
free(*b);
return ERROR_CONDITION;
} else {
return SUCCESS;
}
}

since at the end of my_func() you could be accessing a pointer (b) that's
already been free-ed. And that's just this special case where it's safe to call
the "dealloc" function (free()) when the "alloc" function (malloc()) fails.

So, I don't agree that it's usually easy to determine what needs to be undone
and, even when you can, it's often inefficient compared to having a specific
variable that TELLS you what needs to be undone and so gainsays the slight
performance advantage introduced by the goto.
And given the triviality of the example, it can be rewritten without goto
and without nested if's:

a() == OK && b() == OK && c() ;

but this is not the point. I'd rather use the goto version, BTW ;-)

I'd guess we may be about one or 2 postings away from "let's agree to disagree" ;-).

Don't get me wrong, I do think that using gotos can improve the readability of
convoluted code, I just think that if you go into it assuming that you'll use
gotos then you'll miss an opportunity to do an even better job. FWIW, a non-goto
version of the OPs function would be:

int my_func() {
enum { INIT, GOT_A_MEM, DID_A, GOT_B_MEM } rslt = INIT;
char *a, *b;
int x, ret = ERROR;

a = malloc(some_number);
if (a != NULL) rslt = GOT_A_MEM;

if (rslt == GOT_MEM) {
x = another_function(a);
if (x != ERROR_CONDITION) rslt = DID_A;
}

if (rslt == DID_A) {
x = func_that_mallocs(&b);
if (x != ERROR_CONDITION) rslt = GOT_B_MEM;
}

switch(rslt) {
case GOT_B_MEM: ret = NO_ERROR; free(b);
case DID_A:
case GOT_A_MEM: free(a);
}

return ret;
}

which isn't significantly longer or more highly indented than the goto-version
but doesn't have the UB problem and would work in the general case of any
resource that can only have it's "dealloc" function called if it's associated
"alloc" had succeeded.

Regards,

Ed.
 
C

Christian Bau

"E. Robert Tisdale said:
You are so correct.


This is a good example of *very* poor programming practice.

Are you trolling?
int my_func(void) { // returns NO_ERROR or ERROR
int error = N0_ERROR; // default return value
char* a = (char*)malloc(some_number);
if (NULL != a) {
int x = another_function(a);
if (ERROR_CONDITION == x) {
error = ERROR;
}
else { // ERROR_CONDITION != x
char* b = NULL; // define variables & constants
// close to first use.
int x = func_that_mallocs(&b);
if (ERROR_CONDITION == x) {
error = ERROR;
}
free(b); // free in same scope as malloc
}
}
free(a); // free in same scope as malloc
return error; // single exit point
}

1. C functions should have a single exit point.
2. Don't declare undefined variables.
3. Place veariable and constant definitions
as close to first use as possible.
4. Don't declare variables in an enclosing scope
just to "reuse" the memory allocated for them.
5. Free allocated memory as soon as possible.

Mindless obeyance to rules creates bad code, as you demonstrate
perfectly well.
 
R

Richard Heathfield

Jirka said:
E. Robert Tisdale wrote:
...

if ((f = fopen(fn, "r")) == 0) {
fprintf(stderr, "error opening %s\n", fn);
return 0;
}

Are you suggesting that the return statement
should be replaced by a goto? ;-)

Disclaimer: It's not my fault if Tisdale happens to agree with something
I've said many times in the past.

I don't think a goto is necessary. I would suggest that the code /could/ be
replaced by something along these lines:

fp = fopen(fn, "r");
if(fp != NULL)
{
rc = foo(fp);
fclose(fp);
}
else
{
rc = CANNOT_OPEN_FILE;
}

return rc;
}
 
C

Christian Bau

Ed Morton said:
int my_func() {
enum { INIT, GOT_A_MEM, DID_A, GOT_B_MEM } rslt = INIT;
char *a, *b;
int x, ret = ERROR;

a = malloc(some_number);
if (a != NULL) rslt = GOT_A_MEM;

if (rslt == GOT_MEM) {
x = another_function(a);
if (x != ERROR_CONDITION) rslt = DID_A;
}

if (rslt == DID_A) {
x = func_that_mallocs(&b);
if (x != ERROR_CONDITION) rslt = GOT_B_MEM;
}

switch(rslt) {
case GOT_B_MEM: ret = NO_ERROR; free(b);
case DID_A:
case GOT_A_MEM: free(a);
}

return ret;
}

which isn't significantly longer or more highly indented than the
goto-version
but doesn't have the UB problem and would work in the general case of any
resource that can only have it's "dealloc" function called if it's associated
"alloc" had succeeded.

Introducing an enum and a switch statement is completely unnecessary and
only complicates matters for no gain in readability at all. Consider
this version instead:

int my_func() {
char *a, *b;
int x, ret = ERROR;

a = malloc(some_number);
if (a == NULL) goto FAILED_A_MEM;

x = another_function(a);
if (x == ERROR_CONDITION) goto FAILED_A;

x = func_that_mallocs(&b);
if (x == ERROR_CONDITION) goto FAILED_B_MEM;

ret = NO_ERROR;

free(b);
FAILED_B_MEM:
FAILED_A:
free(a);
FAILED_A_MEM:
return ret;
}
 
N

Nick Keighley

Sheldon Simms said:
Yes yes, thanks. I've read them before.

ah. It was just you said/implied in your original post something along
the lines of "some professor" said we shouldn't use goto's. I thought
Dijkstra
rated more consideration than that. And he puts the case better than
anything else I've read. Though you have to put it in context; when he
said it, it was arguing against a more pro-goto culture.
But Dijkstra isn't God, and "Go To Considered Harmful" isn't
holy scripture.
blasphemy!

As for Knuth, he's *for* goto, as long as
it's properly used.

that's why I posted the link. The point (as others in this thread have
said) is "properly used".

So once again, what would be the advantage of eliminating
goto in the code under discussion (which I reinsert below)?

I have no burning urge to strip out the gotos. In fact I quite like
the
code.


--
Nick Keighley

I mean, if 10 years from now, when you are doing something quick and
dirty,
you suddenly visualize that I am looking over your shoulders and say
to
yourself, "Dijkstra would not have liked this", well that would be
enough
immortality for me.
-- Edsger W Dijkstra
 
J

Jirka Klaue

Richard said:
I don't think a goto is necessary. I would suggest that the code /could/ be
replaced by something along these lines:

fp = fopen(fn, "r");
if(fp != NULL)
{
rc = foo(fp);
fclose(fp);
}
else
{
rc = CANNOT_OPEN_FILE;
}

return rc;
}

IMO this method has two drawbacks. It leads to functions doing
little with *many* lines. In case of more preparation (more files,
some mallocs, sockets, whatever) it gets messy pretty quick. The
method does not scale very good.

Jirka

PS: Didn't you promise, you will not get involved in a style discussion
anymore? ;-)
 
E

Ed Morton

Christian Bau wrote:
Introducing an enum and a switch statement is completely unnecessary and
only complicates matters for no gain in readability at all.

I'd say the opposite - that introducing multiple goto destinations is
completely unnecessary and only complicates matters for no gain in
readability at all when you can use a local variable and a switch statement.

Consider
this version instead:

int my_func() {
char *a, *b;
int x, ret = ERROR;

a = malloc(some_number);
if (a == NULL) goto FAILED_A_MEM;

x = another_function(a);
if (x == ERROR_CONDITION) goto FAILED_A;

x = func_that_mallocs(&b);
if (x == ERROR_CONDITION) goto FAILED_B_MEM;

ret = NO_ERROR;

free(b);
FAILED_B_MEM:
FAILED_A:
free(a);
FAILED_A_MEM:
return ret;
}

Yeah, I thought about that too, but as Dan mentioned:
If all the gotos in a function have the same destination,
few people have a valid reason for complaining.

In the code you just posted, the gotos all have different destinations
so I didn't want to suggest that that's the type of code Dan would write
given his explicit desire to avoid doing that.

What if the function had 10 failing conditions/cleanups? Would you
really introduce 10 goto destinations?

What if you couldn't use a simple fall-through at the end to release
resources? With my version you can test the variable as often as
necessary for whatever combination of values are appropriate, e.g. you
could do this instead of a "switch":

if (rslt == A || rslt == B) {
release(x);
}
if (rslt == B || rslt == C) {
release(y);
}

whereas with yours the equivalent would at it's most readable be
something like:

A: release(x);
goto END;
B: release(x);
release(y);
goto END;
C: release(y);
goto END;
END: return;

i.e. nested gotos and duplicated code.

Even regardless of readability, think about debugging your version
versus mine. In mine, at the end of the function you could just dump the
"rslt" variable and know how the execution went, whereas in yours .....

So, while it doesn't look too bad for this particular function, I don't
think that in general having multiple gotos is a better alternative to a
variable tracking progress.

Ed.
 
J

Jirka Klaue

Ed said:
whereas with yours the equivalent would at it's most readable be
something like:

A: release(x);
goto END;
B: release(x);
release(y);
goto END;
C: release(y);
goto END;
END: return;

switch (rc) {
A: release(x); break;
B: release(x);
C: release(y);
}
 
T

The Real OS/2 Guy

On Wed, 22 Oct 2003 07:06:27 UTC, Christian Bau

some non-transparent code.

I've learned in my first days of programming (the dark age where not
even assembly was available) that

- making a well design is more than the half work.
- goto is evil and should be avoided whenever possible
(o.k., in very low language as mashine code
or assembly it is truly impossible but for that
you can always design the code so that there is no
long goto.
And with C you can design the code always in a
manner that NO goto is needed. Make the design
right and you gets always transparent code.
- find a time inexpensive algoritm before you starts coding
to reduce runtime when a loop tends to be time expansive
That means: start to optimise time before the first statement
gets written.


Good design in programming level is:

- a function has only one return in either success or fail
The same function can have many return in the contrary case.
That means: when the natural result of a function is FAIL
then only ONE exit that flags the fail exist, but multiple
returns flagging success may exist anyway.
When the natural meaning of a function is success then only
ONE return that flags the success exist, but multiple
return flagging an error can exist.
- document each function well. This is needed for maintenance anyway,
even when you are the only for eternity who will own the source.
This will save you moth of maintenance.
document the interface of each funtion well. This will
save you time in using it.
- a fuction exeeding substantial more than one screen page
is buggy per design.
- nesting more than 3 blocks (conditional, loops) tends to be
errornous
break them down in nested fuctions increases the readability.
- breaking nested loops into nested functions helps in readability
because a return error/successcode says more than a goto can ever
say.
- whenever your compiler is unable to support _inline_ use the
preprocessor to define micro steps who are in themself not clearly
writeable, because a self speaking function/macro name tells more
than a non-transparent statement ever can. This increases the
maintenanceability significantly.
- define macros who are object of changeability globally
- define macros you creates only for increasing the readability of
code near the place(s) you need them. Don't forged to #undef them.
- avoid casting. Recheck the design before you tries to cast!
cast only when you absolutely and exactly knows what you are doing,
cast never only to get the comiler quiet without knowing
exactly why it whines and you're not 100% sure that you knows what
you're doing is better than the comiler knows. Be sure, commonly
the compiler knows better than you why it is crying.

Whenever a fuction tends to be bigger than a screen page (about 50
lines), break it down in multiple functions:
- describe WHAT is to do in higher level and HOW is it done in lower
level
- break WHAT is to do from overview to more detailed.
- break HOW is to do to WHAT is to do until you gets fine details
- break HOW is to do in more fine details
- don't nest loops with conditions inside - make own functions out of
them.
This increases the readability and makes it easier to debug
Whereas details means ownstanding functions. It is noways a disgrace
to write a function that gets called once. Because that function helps
you to write readable, clean code as it hides details in the flow and
tells by its name what is to be done.

A side effect of the above would be that you avoids goto, increases
the readability and maintenanceability significantly.

Again: prefere design over coding! Code only when the design is
crlean, checked and rechecked.

At compile time use the most detailed warning level - always! Because
the compier knows more about C as you'll ever be learn when you're not
a compiler developer. Letz the compiler help you to find unsave,
unsecure, unstable code! Again: never suppress a warning by casting
except you have checked and rechecked that you exactly knows that you
knows that the compiler can't know something you knows and you knows
here that even as the compiler means you may loose some data you knows
that it is impossible because you knows the min/max occures here fits.
 
E

Ed Morton

Jirka said:
switch (rc) {
A: release(x); break;
B: release(x);
C: release(y);
}

Sorry it wasn't clear - the A, B, and C in this case are goto labels.
That type of switch would work for the preceeding case where I was using
a variable to hold results though if you prefer that to the if
statements and don't mind duplicating the call to "release(x);".

Ed.
 
J

Jirka Klaue

Ed said:
Sorry it wasn't clear - the A, B, and C in this case are goto labels.

It was clear. What I want to express is that there is no *single* approach.
Just because you can write everything without goto (or switch) doesn't mean
that this will always lead to the most readable code.

Jirka
 
D

Dan Pop

OK, then how would you approach the OPs specific function? I'd expect your
answer to look like this:

int my_func() {
char *a, *b;

At least b MUST be initialised as a null pointer, otherwise a failed
malloc call would result in free() being called on an uninitialised
pointer.
int x, ret = ERROR;

a = malloc(some_number);
if (a == NULL) goto cleanup;

x = another_function(a);
if (x == ERROR_CONDITION) goto cleanup;

x = func_that_mallocs(&b);
if (x == ERROR_CONDITION) goto cleanup;

ret = NO_ERROR;

cleanup:
free(b);
free(a);

return ret;
}

but this in theory invokes undefined behavior if func_that_mallocs() is written
as (pseudo-code):

int func_that_mallocs(char **b) {
*b = malloc(...);
.... /* ensure b is not NULL and do other things */
if (today_is_tuesday) {
free(*b);
return ERROR_CONDITION;
} else {
return SUCCESS;
}
}

since at the end of my_func() you could be accessing a pointer (b) that's
already been free-ed. And that's just this special case where it's safe to call
the "dealloc" function (free()) when the "alloc" function (malloc()) fails.

This is a highly artificial example. There is NO point in designing
func_that_mallocs that way. If it *has* to free *b, then it can also
trivially set it to NULL after calling free on it. The value of b in
my_func() MUST NOT be indeterminate after calling func_that_mallocs.

Dan
 
C

CBFalconer

Nick said:
.... snip ...


that's why I posted the link. The point (as others in this
thread have said) is "properly used".


I have no burning urge to strip out the gotos. In fact I quite
like the code.

Once more, although I am an advocate of suitable use of goto, this
is not one of them. I would write:

int do_stuff(char * iname, char * oname)
{
int rv; /* the error return value */
FILE *inf;
FILE *outf = NULL;
void *mem = NULL;

if (!(inf = fopen(iname,"r")) rv = ERROR;
else if (!(outf = fopen(oname,"w"))) rv = ERROR;
else if (!(mem = malloc(WHATEVER_SIZE))) rv = ERROR;
else {
/* do stuff with inf, outf and mem */
rv = 0;
}
free(mem);
if (outf) fclose(outf);
if (inf) fclose(inf);
return rv;
}

which I believe has similar semantics, less code, fewer variables,
and more clarity. It does not have the memory leak, nor the
failure to close the files on success. It still lacks checking
for fclose success.
 
S

Sheldon Simms

On Wed, 22 Oct 2003 07:06:27 UTC, Christian Bau

some non-transparent code.

Actually it was completely obvious what was going on there.
If you were confused by it, perhaps you should give up on
programming.
I've learned in my first days of programming (the dark age where not
even assembly was available) that

Oh really? You've been programming since before assembly
was available? When was that then, 1948?
- making a well design is more than the half work.
- goto is evil and should be avoided whenever possible

Were they teaching that in 1948? I didn't know that.

In any case, you are wrong. goto is not evil.
You can do evil things with goto, but that's not
the same as goto being evil.

I could drive at high speeds through the middle of the
pedestrian zone in downtown Cologne at noon on Saturday,
but that doesn't make cars evil.
And with C you can design the code always in a
manner that NO goto is needed.

True, but there are also times in C programming when goto is
an appropriate construct to use.
- a function has only one return in either success or fail
The same function can have many return in the contrary case.
That means: when the natural result of a function is FAIL
then only ONE exit that flags the fail exist, but multiple
returns flagging success may exist anyway.
When the natural meaning of a function is success then only
ONE return that flags the success exist, but multiple
return flagging an error can exist.

Herbert, erbarme dich unser.
- a fuction exeeding substantial more than one screen page
is buggy per design.

Du nimmst hinweg die Sünde der Programmierer. Erbarme dich unser.
- nesting more than 3 blocks (conditional, loops) tends to be
errornous break them down in nested fuctions increases the
readability.

A goto can work here too.
- breaking nested loops into nested functions helps in
readability because a return error/successcode says more than
a goto can ever say.

Ah, but what if there is no chance for "failure" in a given nested
loop? What's the point of returning an "error/successcode" then?
Whenever a fuction tends to be bigger than a screen page (about 50
lines), break it down in multiple functions: - describe WHAT is to do in
higher level and HOW is it done in lower level - break WHAT is to do
from overview to more detailed. - break HOW is to do to WHAT is to do
until you gets fine details - break HOW is to do in more fine details -
don't nest loops with conditions inside - make own functions out of
them.

Herbert sei dank!
This increases the readability and makes it easier to debug
Whereas details means ownstanding functions. It is noways a disgrace to
write a function that gets called once. Because that function helps you
to write readable, clean code as it hides details in the flow and tells
by its name what is to be done.

Das Evangelium nach Herbert
A side effect of the above would be that you avoids goto, increases the
readability and maintenanceability significantly.
Amen.

Again: prefere design over coding! Code only when the design is crlean,
checked and rechecked.

At compile time use the most detailed warning level - always! Because
the compier knows more about C as you'll ever be learn when you're not a
compiler developer. Letz the compiler help you to find unsave, unsecure,
unstable code! Again: never suppress a warning by casting except you
have checked and rechecked that you exactly knows that you knows that
the compiler can't know something you knows and you knows here that even
as the compiler means you may loose some data you knows that it is
impossible because you knows the min/max occures here fits.

Ehre sei dir, O Herbert.
Amen.
 
E

Ed Morton

Dan Pop wrote:

This is a highly artificial example. There is NO point in designing
func_that_mallocs that way. If it *has* to free *b, then it can also
trivially set it to NULL after calling free on it.

I think it's a pretty reasonable example - if you're wrapping malloc (or
any other resource-allocating primitive) in a function, presumably it's
doing more than just malloc-ing the memory, and if it returns failure
I'd hope it releases any resources it did succesfully grab before
returning. The only real question is whether or not it should
additionally set *b to NULL before return but:

a) although UNLIKELY to create a real problem, according to the FAQ
(http://www.eskimo.com/~scs/C-faq/q7.21.html), accessing a pointer after
free-ing it produces implementation-defined behavior, and
b) the only reason to do that is because the calling function now
depends on it, which is data-coupling and so to be avoided.

The value of b in
my_func() MUST NOT be indeterminate after calling func_that_mallocs.

I'd say that the calling function MUST NOT make any assumptions about
the value of b when func_that_mallocs() returns failure. I'd also say
that free(NULL) is a special case anyway and that in general you
shouldn't assume you can unconditionally call the "dealloc()" function
if the "alloc()" one failed (as you pointed out with fclose()).

Ed.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,581
Members
45,056
Latest member
GlycogenSupporthealth

Latest Threads

Top