use assert and defensive together

[Richard Tobin]

Which is an advantage of letting the system handle it. In the bad old days
illegal memory writes would cause funny things to happen to the program. Now
they usually cause an error mesage and program termination.

What bad old days were those? Were you using C 30 years ago, or do
you mean the "bad fairly recent days" when people ran C on the
pathetic operating systems provided by Apple and Microsoft before they
were finally dragged into the 1970s, 25 years late?

-- Richard

 

[Flash Gordon]

Malcolm McLean wrote, On 24/09/08 23:08:

Your program is bugged. Whose to say that the alarm triggering code will
function as designed under such circumstances?

Never heard of watchdogs? These are often HW devices that trigger when
the SW has crashed and stops keeping them happy. Also I *have* written
SW that triggers on a crash, and if you program defensively you can
trigger it before the problem is bad enough to actually crash the SW!

Whilst you can catch a lot of bugs at stage 1, when you are running the
code through the debugger, eventually you've got to release the code
without. Often to some internal QA person.

So how are you going to debug the released code which does *not* have
defensive code? Why is the lack of defensive code making it easier to
debug, which was you claim that I was addressing here? Also the logging
would still be available in the released code as will the reports to the
user. Or are you saying that you cannot understand a bug report from a
user which says, "The SW reported that it cannot save an image with a
negative size!"

It make the code dependent on the logging system. None of the code on my
website could be used if I employed such a system, for instance.

No it doesn't, and I provided part of a sample interface for you which
keeps them nice and separate and allows you to use the code *without*
the logging system if that is your choice.

Yes, if you are effectively writing one program / library then it solves
a lot of software engineering problems. The interfaces don't need to be
as clean, you can write errors to special log files, etc.

Try reading what I wrote. The library is NOT used for just one program.
It is used for a number of programs written by independent companies.

We've been through this. Where human back-up isn't available, defensive
programming is the best that you can do. Generally you don't want it. No
results are usually better than wrong results.

You still don't get it. Defensive programming can prevent you from
getting wrong results. Depending on requirements it can do this by
preventing you from getting any result other than a bug report, or
sometimes it can deal with the problem in such a way that the program
can still produce the correct results (or some of the correct results
and flag where results are not available due to the problem). In no
instance is defensive programming actually *required* to hide that there
is a problem.

Not always, because then you can clutter the program with error
messages,

Or you can do the job properly so the program is still not cluttered.

increasing maintenance costs, decreasing readability, and
increasing the bug count.

I've seen one instance where incorrect defensive code increased the bug
count. The defensive code also produced a report that told us what was
wrong and allowed us to fix the problem in very short order. I've seen
lots of instances where lack of defensive code made it very hard to find
the bug.

Generally, let the system seg fault, and don't try to sanity check
length parameters and the like.

So tell me, which of these bug reports will make it easier to debug...

1) The program crashed with a seg fault.
2) The program gave a screwy message saying it could not save a negative
sized image!
3) The program crashed saying it could not allocate 32GB of memory, the
picture was only 3x3 pixels!!!
4) The program crashed saying the foo stack was corrupted!

My experience of all these types of things is that 1 is by far the
hardest to debug where as the defensive code generating the others makes
it easier since you know exactly where the crash occurred and if you
can't see the problem by reading the code you can stick a breakpoint on
the trap with a debug version of the code, or add in more logging around
the calls to the function, or anything else that suits since you have a
starting point.

Which is an advantage of letting the system handle it.

No, it was the advantage of me adding defensive code to trap yet another
error condition and handle it.

In the bad old
days illegal memory writes would cause funny things to happen to the
program.

Personal experience. The bad old days include today.

Now they usually cause an error mesage and program termination.

Rely on that if you want, but defensive code handles the situation even
earlier, closer to the point of error and in a more controlled manner
which can, if appropriate, include aborting the program with a more
useful message than a SIGSEGV. Also it can save the possibly corrupted
file to a different name for later attempted recovery if that is
appropriate.

 

[Flash Gordon]

Malcolm McLean wrote, On 26/09/08 21:20:

Duh. Image-saving I meant. Image loading is easily defended agaisnt by
returning a NULL for failure to load the image.

Well, on your savejpeg.c and bmp.c files you can use the following in
your save routines:
if (height<0 || width<0 || path || *path || rgb)
return -1;

You might want to also deal with fputc failing. After all, even on
modern systems disks fill up occasionally!

After all, the caller must be dealing with the -1 error code generated
if the function fails to open the file. It would be better if you used
different return codes for each type of failure, but I'm not going to
fix al of your code. You could also do what I suggested and provide a
mechanism to register a log function and call that if it is passed
invalid parameters.

Oh, and you should use a compiler with a decent warning level on it. You
maketables function does not use any of the parameters it is passed. The
function is therefore incorrect because it should use them or incorrect
because it takes them.

 

[Flash Gordon]

Malcolm McLean wrote, On 28/09/08 18:16:

The problem is that -1 is the return for the operation failing, due to
failure to open the file.

It is also used for several other error conditions.

I take the point that it would be better to
check every call to fputc as well.
Good.

You are adding error returns for the calling program being incorrect.

As one of several things.

That's what I mean by bug hiding.

No, because the calling program is notified and will report to the user
that it failed and if the calling program is any good at all will report
the reason for the failure.

You are also completely ignoring the suggestion of logging for which I
provided an easy solution earlier.

Sometime you do want to do it, for
instance you might want to hide from the user the fact that the program
is bugged. However generally you don't. You want the system to take
action against the program so that caller realises he has made a mistake.

Call abort after providing an appropriate error message if you prefer.

As for a list of error returns, that isn't such a good idea.

So you like the idea of the same error message being given to the user
regardless of the error? In which case I suggest you rewrite the copy of
gcc you use so that for any error it just reports "there is an error
somewhere but I'm not telling you what or where".

Caller then
has to say

switch( savejepg(rgb, width, height) )
{
case JPEG_ERROR_OPENING_FILE:
case JPEG_PARAMSNEGATIVE:
case JPEG_RGBPARAMZERO:
}

No, you can group error codes and provide error analysis functions for
when they are required. The simplest error analysis function being a
look up of the error code in a list of strings. In any case, a save
function that provides no means to identify the cause of failure is of
no use for serious work anyway because users always want to know *why*
the save failed.

what we gain is that we remind the caller that width and height may not
be negative.

No, you also get the caller reporting up the line eventually to the user
that the save failed.

But the cost in ease of use is unacceptable. All the
additional error-checking code will increase our bug count, simply by
blowing up the number of lines and branch points. It's not easy to
measure by how much, but that doesn't mean the effect is unimportant.

There is a case for keeping all the errors negative, and allowing

if(savejpeg(rgb, width, height < 0)
/* error saving file */

That's not too much of a burden. But by confusing programming errors
with incorrect paths, you are bug hiding.

Compete rubbish. Somewhere in the error handing code you will have a
call to an error decode function and end up telling the user that it
failed due to an invalid image size. The user will then (depending on
the situation) either try something else or report the bug to you if
they know the size is valid. Look at the standard library functions such
as "strerror" for ideas. I.e. the call would be something more like:

result = savejpeg(rgb, width, height < 0);
if(result < 0)
fprintf(stderr, "Error (%d) saving file: %s\n", result,
jpegerr(result));

Not hard at all. Of course, you have to do what you had failed to do and
separate out the various error codes.

I, at least, would want to know if saving failed due to inability to
open the file, insufficient memory (you return -1 if there was a malloc
failure in your save code as well), insufficient space or some other
problem. jpegerr could, of course, use strerror in constructing the error.

maketables has a comment above it expalining why the parameters are
unused. This is more fully explained in the text of the book.
Essentially savejpeg() is a very simple JPEG encoder, and this function
can be expanded to improve the compression ratio, by returning a set of
tables based on image characteristics.

Add the parameters when you need them. Until you have designed the code
to do more complex stuff you don't know what you will be passing in. You
might add a function for image analysis and pass in the structure that
provides instead of passing in the image.