[comp.sys.amiga.tech] More suggestions for programmers

SLMYQ@USU.BITNET (04/06/88)

Here's another suggestion for programmers.  It basically has to do with
freeing resources you've allocated, EVEN WHEN THE PROGRAM HAS FAILED.

The majority of programs I've seen just do something like this:

main()
{
        if ((Screen = OpenScreen(&NewScreen)) == NULL)
                exit(1);

        if ((Window = OpenWindow(&NewWindow)) == NULL)
                exit(2);

etc.

Now here's the problem.  Although in this example it would be unlikely, it
is possible that the Screen will open correctly but the Window won't.  Then
the program exits with a return code of 2, leaving the screen wide open and
a very confused user.

Although one solution would be to have every allocation call, on an error,
have all the commands needed to free everythings that was allocated before.
However, this quickly gets not-so-fun:

        if (OpenDevice("abc.device",0,AbcReq,0) != 0) {
                DeleteExtIO(AbcReq);
                DeletePort(AbcPort);
                ClearMenuStrip(Window);
                CloseWindow(Window);
                CloseScreen(Screen);
                exit(1);
        }

You get the point.  Now here's the solution I always use.

struct Window *Win;
struct Screen *Scr;
struct IOStdReq *AbcReq;
struct MsgPort *AbcPort;

VOID Finish(Mes)
TEXT *Mes;
{
        if (AbcReq != NULL) {
                if (AbcReq->io_Device != NULL)
                        CloseDevice(AbcReq);
                DeleteStdIO(AbcReq);
        }
        if (AbcPort != NULL)
                DeletePort(AbcPort);
        if (Win != NULL) {
                ClearMenuStrip(Win);
                CloseWindow(Win);
        }
        if (Scr != NULL)
                CloseScreen(Scr);

        if (Mes != NULL) {                      /*********************/
                if (stdout != NULL)
                        printf("%ls",Mes);      /* Make this a macro */
                exit(RETURN_ERROR);
        } else
                exit(RETURN_OK);                /*********************/
}

There are a few things I left out, but I'm leaving that to you.  It really
isn't as bad as it looks especially (1) you can put the marked part into
a standard macro, so that whole part just looks like, say, "FinExit(Mes)".
And for the first part, the checking for zero lines are nothing compared to
the countless times you might have to repeat each thing if you repeated
the process everywhere that might fail.  This way, if something goes wrong
somewhere, all you have to do is say

        Finish("Not Enough Memory\n");

or something like that.  You might even want to put the above statement into
it's own function called FinNoMem() or something of that nature, since it's
likely the most common error message.

Now here's a small side topic.  Notice the newline character at the end of
the Not Enough Memory message above.  ALWAYS END YOUR LINES WITH A NEWLINE
unless you really want to put something after it on the line.  Otherwise
the AmigaDOS prompt will come after the message on the same line:

1>ABC
Not Enough Memory1>

Doesn't look too nice.

One last side dish.  This concerns return codes.  I've seen all sizes and
shapes of return codes, even negative ones.  Now, in the AmigaDOS manual
(somewhere :), it states that programs should return one of FOUR possible
return codes - 0, 5, 10, and 20.  That's IT.  Nothing else.  0 is for OK,
5 is for just a warning message (this file has lines that are too long and
they got chopped...), 10 is for an error (file not found), 20 for a complete
and total failure (no version of Intuition is found in the system :)
Now if you'd *PLEASE* stick to the standards AmigaDOS laid out, things would
be much nicer - even if you don't think highly of AmigaDOG :)

Now that you've listened to all my mumbo jumbo, please don't take it that I
want to be the big dictator who says how things should be done.  I just want
to clarify a few points that might make programs better in general.

                                Bryan

        Bryan Ford                    //// A computer does what \\\\
Snail:  1790 East 1400 North         //// you tell it to do, not \\\\
        Logan, UT 84321          \\\X///  what you want it to do. \\\X///
Email:  SLMYQ@USU.BITNET          \XXX/ Murphy's Law Calendar 1986 \XXX/

john13@garfield.UUCP (John Russell) (04/11/88)

In article <8804060758.AA14048@jade.berkeley.edu> SLMYQ@USU.BITNET writes:
>Here's another suggestion for programmers.  It basically has to do with
>freeing resources you've allocated, EVEN WHEN THE PROGRAM HAS FAILED.
>
>The majority of programs I've seen just do something like this:
>
>main()
>{
>        if ((Screen = OpenScreen(&NewScreen)) == NULL)
>                exit(1);
>
>        if ((Window = OpenWindow(&NewWindow)) == NULL)
>                exit(2);
>
>Although one solution would be to have every allocation call, on an error,
>have all the commands needed to free everythings that was allocated before.
>However, this quickly gets not-so-fun:
>
>You get the point.  Now here's the solution I always use.
>
[ example of Finish() which checks all resources and frees if they have been
  allocated ]

This is a good point. People using Manx will probably want to call this routine
_abort() so that it will be automatically called on ^C interrupt. Then you
can check for ^C yourself or just do periodic calls to the Manx printing
functions (which do it themselves).

I've used this approach to allow aborts which happened while the WB screen
was dual-playfield and had a picture halfway loaded in, or windows belonging
to other applications were locked and having their contents dumped to the
printer, without difficulty.

Things to watch for:

- if you lock layers, have a boolean variable (eg layers_locked = FALSE) that
  lets your _abort() routine know it has to unlock them; some resources may
  not have a pointer associated with them
- make sure everything is initialized to NULL, FALSE, etc. Whatever state
  shows that it isn't currently in use.
- make sure when you free up something inside the program you reset the state
  of that resource to be false, eg

fp = fopen(...)
[ read & write ]
fclose(fp);
fp = NULL;

...

_abort()
{
	if (fp) fclose(fp);
...
}

John
-- 
"Bagpipers are prone to lung infections due to fungus that grows inside the bag"
				- Time magazine brightens my day

eric@cbmvax.UUCP (Eric Cotton) (04/13/88)

In article <8804060758.AA14048@jade.berkeley.edu> SLMYQ@USU.BITNET writes:
>[...lottsa good stuff deleted...]
>One last side dish.  This concerns return codes.  I've seen all sizes and
>shapes of return codes, even negative ones.  Now, in the AmigaDOS manual
>(somewhere :), it states that programs should return one of FOUR possible
>return codes - 0, 5, 10, and 20.  That's IT.  Nothing else.  0 is for OK,
>5 is for just a warning message (this file has lines that are too long and
>they got chopped...), 10 is for an error (file not found), 20 for a complete
>and total failure (no version of Intuition is found in the system :)
>Now if you'd *PLEASE* stick to the standards AmigaDOS laid out, things would
>be much nicer - even if you don't think highly of AmigaDOG :)

Specifically, the return codes are as follows (from dos.h):

#define RETURN_OK                           0  /* No problems, success */
#define RETURN_WARN                         5  /* A warning only */
#define RETURN_ERROR                       10  /* Something wrong */
#define RETURN_FAIL                        20  /* Complete or severe failure*/

-- 
	Eric Cotton
	Commodore-Amiga

  *======================================================================*
 *=====     UUCP: {rutgers|ihnp4|allegra}!cbmvax!eric                =====*
*=====      FONE: (215) 431-9100                                      =====*
*=====      MAIL: 1200 Wilson Drive / West Chester, PA 19380          =====*
 *=====     PAUL: "I don't find this stuff amusing anymore."         =====*
  *======================================================================*