Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Goto still considered helpful

Reply
Thread Tools

Goto still considered helpful

 
 
Ed Jensen
Guest
Posts: n/a
 
      10-15-2007
Peter Pichler <(E-Mail Removed)> wrote:

[SNIP: Examples]

> Which one is more readable?


I actually found your version a little tricky to read. I find
functions more readable if all the clean up happens in one place.

It took me a few seconds to make sure your files were properly closed
no matter what series of events happened.

Using your approach, things would get more tricky pretty quickly if
more resources were added to the function, such as dynamically
allocated read and write buffers.

Also, your approach unnecessarily duplicates code. Even your simple
example contains multiple calls to close the very same file.

I like the following pattern, which keeps the resource clean up in one
place, and the pattern scales well if more resources are added to the
function in the future:

int fcopy(char const *dst, char const *src)
{
FILE *sf = NULL;
FILE *df = NULL;

sf = fopen(src, "r");
if (!sf)
goto done;

df = fopen(dst, "w");
if (!df)
goto done;

/* copy happens here */

done:
if (sf)
fclose(sf);
if (df)
fclose(df);

return /* value indicating success or failure */ 0;
}
 
Reply With Quote
 
 
 
 
inmatarian
Guest
Posts: n/a
 
      10-15-2007
Kenneth Brody wrote:
> Well, now you get into the "multiple returns" scenario, which is
> met with as much disdain (if not more) than the use of goto.
>
> Imagine extending your function to one which also did several
> malloc()s for buffers, with each failure point now needing to do
> cleanup of those things which succeeded before it. (ie: if the
> first malloc fails, fclose the two streams; if the second malloc
> fails, fclose the streams and free the first malloc; if the read
> of the header fails, fclose the streams and free both buffers; and
> so on.)
>

Granted, although you would have a big group of fcloses and frees in any
of the forms of this particular function. I'm not saying either way is
particularly better, and I'm sure gotos have their place, but I'd
recommend deferrence to Eric S Raymond's rules, or whoever your favorite
pioneer is, they all said basically the same thing. In this case, I'm
citing two rules:

Rule of Repair: When you must fail, fail noisily and as soon as possible.

Rule of Modularity: Write simple parts connected by clean interfaces.

Having three returns in a file copy function isn't all that bad, though
I'd agree that when you start pushing it on multiple returns, that
probably fails Rule of Modularity, in that the function is doing too much.
 
Reply With Quote
 
 
 
 
Peter Pichler
Guest
Posts: n/a
 
      10-15-2007
Ed Jensen wrote:

> done:
> if (sf)
> fclose(sf);
> if (df)
> fclose(df);


As you wish. I do not like this approach because it requires all
declarations at the top of the function, which IMO is almost as bad
as making them global. In addition, it requires them initialised
to "harmless" values which can hide bugs later on. Of course, YMMV.
De gustibus non disputandum est.
 
Reply With Quote
 
CBFalconer
Guest
Posts: n/a
 
      10-15-2007
Kenneth Brody wrote:
> inmatarian wrote:
>
> [... nested-if versus goto ...]
>
>> Is there a problem with the Immediately Crash principle? I could
>> write your function like so:
>>
>> int fcopy (char const * dst, char const * src)
>> {
>> FILE * sf /*= NULL? yuck!*/;
>> FILE * df /*= NULL? yuck!*/;
>>
>> sf = fopen(src, "r");
>> if (!sf)
>> return /* value indicating failure */ 1;
>>
>> df = fopen(dst, "w");
>> if (!df)
>> {
>> fclose(sf);
>> return /* value indicating failure */ 1;
>> }
>>
>> /* "meat" of the intended file copy function
>> omitted for clarity */
>>
>> fclose(df);
>> fclose(sf);
>> return /* value indicating success */ 0;
>> }
>>
>> I think a good lesson from all of this would be to recognize the
>> need for a goto statement as a sign your function is doing too
>> much, instead of the Do One Thing Well principle.

>
> Well, now you get into the "multiple returns" scenario, which is
> met with as much disdain (if not more) than the use of goto.


So look at the following, with a single return, and no complexity:

int fcopy(const char *dst, const char *src) {
FILE *sf, *df;
int err, ch;

if (!(sf = fopen(src, "r"))) err = 1;
else if (!(df = fopen(dst, "w"))) err = 2;
else {
while (EOF != (ch = getc(sf))) putc(ch, df);
err = 0;
fclose(df);
}
if (sf) fclose(sf);
return err;
}

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>



--
Posted via a free Usenet account from http://www.teranews.com

 
Reply With Quote
 
Ča\\/b
Guest
Posts: n/a
 
      10-16-2007
In data Mon, 15 Oct 2007 17:09:19 -0000, Ed Jensen scrisse:
>Peter Pichler <(E-Mail Removed)> wrote:
>[SNIP: Examples]
>> Which one is more readable?


my

>I actually found your version a little tricky to read. I find
>functions more readable if all the clean up happens in one place.
>
>It took me a few seconds to make sure your files were properly closed
>no matter what series of events happened.


hahahahha

>Using your approach, things would get more tricky pretty quickly if
>more resources were added to the function, such as dynamically
>allocated read and write buffers.


so not is complete

>Also, your approach unnecessarily duplicates code. Even your simple
>example contains multiple calls to close the very same file.
>
>I like the following pattern, which keeps the resource clean up in one
>place, and the pattern scales well if more resources are added to the
>function in the future:
>
>int fcopy(char const *dst, char const *src)
>{
> FILE *sf = NULL;
> FILE *df = NULL;
>
> sf = fopen(src, "r");
> if (!sf)
> goto done;
>
> df = fopen(dst, "w");
> if (!df)
> goto done;
>
> /* copy happens here */
>
>done:
> if (sf)
> fclose(sf);
> if (df)
> fclose(df);
>
> return /* value indicating success or failure */ 0;
>}


in my almost imagenary language it is 2 lines
int fcopy(char* dst, char* src)
{istream sf(dst); ostream df(src); return (int)(sf>>df);}

the goto should be use to implemet all the rest
while, for, and in general loops are worse than gotos

all in programming area is only use well goto and jumps
 
Reply With Quote
 
Ben Pfaff
Guest
Posts: n/a
 
      10-19-2007
CBFalconer <(E-Mail Removed)> writes:

> int fcopy(char const *dst, char const *src) {
> FILE *sf, *df;
> int ch
>
> if (sf = fopen(src, "r") { /* text files */
> if (df = fopen(dst, "w") {
> while (EOF != (ch = getc(sf))) putc(ch, df);
> fclose(df);
> }
> fclose(sf);
> }
> return sf && df; /* non-zero for success */


I think that this is a bad idea: it is a lot like checking the
value of a pointer after it has been freed (often fclose will
actually call free on the stream, in fact). I'm sure it works in
practice on almost every implementation though.

> }


--
char a[]="\n .CJacehknorstu";int putchar(int);int main(void){unsigned long b[]
={0x67dffdff,0x9aa9aa6a,0xa77ffda9,0x7da6aa6a,0xa6 7f6aaa,0xaa9aa9f6,0x11f6},*p
=b,i=24;for(;p+=!*p;*p/=4)switch(0[p]&3)case 0:{return 0;for(p--;i--;i--)case+
2:{i++;if(i)break;else default:continue;if(0)case 1utchar(a[i&15]);break;}}}
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      10-19-2007
Ben Pfaff <(E-Mail Removed)> writes:
> CBFalconer <(E-Mail Removed)> writes:
>> int fcopy(char const *dst, char const *src) {
>> FILE *sf, *df;
>> int ch
>>
>> if (sf = fopen(src, "r") { /* text files */
>> if (df = fopen(dst, "w") {
>> while (EOF != (ch = getc(sf))) putc(ch, df);
>> fclose(df);
>> }
>> fclose(sf);
>> }
>> return sf && df; /* non-zero for success */

>
> I think that this is a bad idea: it is a lot like checking the
> value of a pointer after it has been freed (often fclose will
> actually call free on the stream, in fact). I'm sure it works in
> practice on almost every implementation though.
>
>> }


If the first fopen() fails, no value is assigned to df, and accessing
it invokes undefined behavior.

In addition, I think you're write; though fclose() cannot change the
value of of its argument, it can cause it to become indeterminate.
(In practice, it will probably appear to be be null or non-null after
fclose() if it was, respectively, null or non-null before fclose().)

--
Keith Thompson (The_Other_Keith) http://www.velocityreviews.com/forums/(E-Mail Removed) <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."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
Reply With Quote
 
CBFalconer
Guest
Posts: n/a
 
      10-19-2007
Ben Pfaff wrote:
> CBFalconer <(E-Mail Removed)> writes:
>
>> int fcopy(char const *dst, char const *src) {
>> FILE *sf, *df;
>> int ch
>>
>> if (sf = fopen(src, "r") { /* text files */
>> if (df = fopen(dst, "w") {
>> while (EOF != (ch = getc(sf))) putc(ch, df);
>> fclose(df);
>> }
>> fclose(sf);
>> }
>> return sf && df; /* non-zero for success */
>> }

>
> I think that this is a bad idea: it is a lot like checking the
> value of a pointer after it has been freed (often fclose will
> actually call free on the stream, in fact). I'm sure it works in
> practice on almost every implementation though.


A point. The alternative is a local 'err' variable. I was mainly
interested in proper error exiting without grotesque code
structure.

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>



--
Posted via a free Usenet account from http://www.teranews.com

 
Reply With Quote
 
CBFalconer
Guest
Posts: n/a
 
      10-19-2007
Keith Thompson wrote:
> Ben Pfaff <(E-Mail Removed)> writes:
>> CBFalconer <(E-Mail Removed)> writes:
>>
>>> int fcopy(char const *dst, char const *src) {
>>> FILE *sf, *df;
>>> int ch
>>>
>>> if (sf = fopen(src, "r") { /* text files */
>>> if (df = fopen(dst, "w") {
>>> while (EOF != (ch = getc(sf))) putc(ch, df);
>>> fclose(df);
>>> }
>>> fclose(sf);
>>> }
>>> return sf && df; /* non-zero for success */
>>> }

>>
>> I think that this is a bad idea: it is a lot like checking the
>> value of a pointer after it has been freed (often fclose will
>> actually call free on the stream, in fact). I'm sure it works in
>> practice on almost every implementation though.

>
> If the first fopen() fails, no value is assigned to df, and
> accessing it invokes undefined behavior.


Note the order of access in the return statement, and use of the &&
operator. So df is not accessed in that case.

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>



--
Posted via a free Usenet account from http://www.teranews.com

 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      10-19-2007
Keith Thompson <(E-Mail Removed)> writes:
[...]
> In addition, I think you're write;

[...]

Did I really write "write" instead of "right"? Sigh.

--
Keith Thompson (The_Other_Keith) (E-Mail Removed) <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."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Why Perl 5.10.0 is still considered stable? howa Perl Misc 8 03-23-2009 10:08 PM
Helpful resources before starting a MCSD course =?Utf-8?B?Q2xvd25zaG9lcw==?= MCSD 1 08-23-2005 06:12 PM
String.intern() still "considered harmful"? Robert Mischke Java 3 05-19-2005 09:15 AM
Paisleyskye, helpful or hinderance Consultant MCSE 31 01-05-2004 09:15 PM
helpful stuff DaPunisher MCSE 0 12-05-2003 04:01 AM



Advertisments