Re: use assert and defensive together

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

"Flash Gordon" <smap@xxxxxxxxxxxxxxxxx> wrote in message
Malcolm McLean wrote, On 26/09/08 21:20:

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.

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.

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.

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.
--
Flash Gordon
If spamming me sent it to smap@xxxxxxxxxxxxxxxxx
If emailing me use my reply-to address
See the comp.lang.c Wiki hosted by me at http://clc-wiki.net/
.

Relevant Pages

  • Re: What to do with a failed automated test
    ... one of then is failing. ... After discussion, the bug is ... report that was produce base on the initial failure. ...
    (comp.software.testing)
  • Re: Initialising Variables
    ... Surely it will lead to incorrect results? ... to compute a new value for it, but you fail to do so (that's the bug. ... subtle failure that's still there in the shipped product). ... It doesn't crash. ...
    (comp.lang.c)
  • Re: New special levels
    ... Incorrect. ... New bugs are fixed quite frequently, and if you report a ... bug to the Dev Team that they haven't seen before, ... shows up on the bug list as 'fixed'. ...
    (rec.games.roguelike.nethack)
  • Re: New special levels
    ... Incorrect. ... New bugs are fixed quite frequently, and if you report a bug ... to the Dev Team that they haven't seen before, ...
    (rec.games.roguelike.nethack)
  • Re: New special levels
    ... Incorrect. ... New bugs are fixed quite frequently, and if you report a bug ... to the Dev Team that they haven't seen before, ...
    (rec.games.roguelike.nethack)