Check all errors in code?

[CBFalconer]

Richard Tobin said:

Call it again. In fact, call it in a loop. The program is bound
to notice eventually.

If exit returns complain to the C system supplier. It is faulty.

 

[lovecreatesbea...]

fclose(NULL) is undefined. These two need to be protected with an if
each.

or better return or exit earlier when a null returned upon fopen.

Such a shame that there is no symmetry with malloc/free but
that is how the world is!

I think the sequence of calls to malloc/free is fine provide NULL
initialization performed before malloc.

 

[lovecreatesbea...]

christian.bau said:



Yours, if you lose your job when the company goes bust after its flagship
product crashes and burns because you wrote shoddy code despite knowing
better.

Aren't there lots of buggy flagship products all around the world?

 

[Keith Thompson]

(e-mail address removed) said: [...]

The following code from net-snmp library calls atoi five times but
checks none.

Feel free to explain how you'd check atoi. Once you've failed to do that,
you will be in a position to explain why the code is broken.

Conceivably the code doesn't need to differentiate between a string
representing 0 and a string that doesn't represent any integer value,
and whatever code calls it will treat either as an error. Or perhaps
the string being passed to atoi() has been pre-checked and the code we
were shown, in context, can never be called with invalid data. It's
not entirely impossible to use atoi() in non-broken code.

It's more likely, though, that the code is indeed buggy.

 

[Ben Bacarisse]

or better return or exit earlier when a null returned upon fopen.

OK, but then you have to free the mem pointers in two places and you
don't get single-entry single-exit functions.

 

[Keith Thompson]

Keith Thompson said: [...]

Conceivably the code doesn't need to differentiate between a string
representing 0 and a string that doesn't represent any integer value,
and whatever code calls it will treat either as an error.

Whilst 0 is a common return value for atoi when its input can't be
represented as an int, the Standard doesn't mandate this, so code that
relies on it is broken.

[...]

Good point; I hadn't realized that.

The following is a paraphrase of C99 7.20.1.2 (dropping the references
to atol and atoll for simplicity):

Description

The atoi function converts the initial portion of the string
pointed to by nptr to int representation. Except for the behavior
on error, it is equivalent to
(int)strtol(nptr, (char**)NULL, 10)

Returns

The atoi function returns the converted value.

And a paraphrase from C99 7.20.1.4:

The strtol function returns the converted value, if any. If no
conversion could be performed, zero is returned.

Until a moment ago, I assumed that the *intent* was that atoi()
returns 0 on error, and the standard just didn't express that intent
properly. But that intent could have been expressed simply by
dropping the phrase "Except for the behavior on error". Since it
doesn't say what the behavior on error *is*, the behavior is
undefined.

A side note: Yes, the (char**) cast on the second argument to strtol
in the definition of atoi is necessary. Even in C99, it's still legal
(though unwise) to call strtol without a visible prototype:

/* no #include <stdlib.h> */
long int strtol();

strtol(nptr, NULL, 0); /* UB */

Here NULL is not of type char**, and it won't be converted to char**,
causing undefined behavior.

 

[vippstar]

Keith Thompson said: [...]

Conceivably the code doesn't need to differentiate between a string
representing 0 and a string that doesn't represent any integer value,
and whatever code calls it will treat either as an error.

Whilst 0 is a common return value for atoi when its input can't be
represented as an int, the Standard doesn't mandate this, so code that
relies on it is broken.

[...]

Good point; I hadn't realized that.

The following is a paraphrase of C99 7.20.1.2 (dropping the references
to atol and atoll for simplicity):

Description

The atoi function converts the initial portion of the string
pointed to by nptr to int representation. Except for the behavior
on error, it is equivalent to
(int)strtol(nptr, (char**)NULL, 10)

Returns

The atoi function returns the converted value.

And a paraphrase from C99 7.20.1.4:

The strtol function returns the converted value, if any. If no
conversion could be performed, zero is returned.

Until a moment ago, I assumed that the *intent* was that atoi()
returns 0 on error, and the standard just didn't express that intent
properly. But that intent could have been expressed simply by
dropping the phrase "Except for the behavior on error". Since it
doesn't say what the behavior on error *is*, the behavior is
undefined.

I believe the "behavior on error" is when the string is not an
integer.
strtol can not invoke undefined behavior. If atoi is equal to that
strtol call, it can't invoke undefined behavior if strtol returns 0.
It will invoke implementation defined behavior or will raise
implementation defined signal (only in C99) if the value returned by

A side note: Yes, the (char**) cast on the second argument to strtol
in the definition of atoi is necessary. Even in C99, it's still legal
(though unwise) to call strtol without a visible prototype:

/* no #include <stdlib.h> */
long int strtol();

strtol(nptr, NULL, 0); /* UB */

Here NULL is not of type char**, and it won't be converted to char**,
causing undefined behavior.

Ah thanks, this bugged me for some time.

 

[CBFalconer]

or better return or exit earlier when a null returned upon fopen.


I think the sequence of calls to malloc/free is fine provide NULL
initialization performed before malloc.

fclose(outfile) can be called with an uninitialized outfile in the
above. So besides protecting by detecting NULL value, you need to
initialized them to NULL.

 

[CBFalconer]

OK, but then you have to free the mem pointers in two places and you
don't get single-entry single-exit functions.

Simply use: if (f) fclose(f);
(assuming no error test on fclose is needed).

 

[CBFalconer]

Whose money is lost if you do shoddy work by following your
bosses orders? If it's your bosses money, go ahead. If it's not
your bosses money, follow your conscience.

Not wise. When the problems arise, who gets blamed and loses
reputation? Just do a proper job in the first place.

 

[Ben Bacarisse]

Simply use: if (f) fclose(f);
(assuming no error test on fclose is needed).

Isn't that what I said?

 

[CBFalconer]

Isn't that what I said?

Yes. I think I meant to point out that the original code could
leave outfile uninitialized, so that was also needed. I then
forgot about it.

 

[Richard Tobin]

[/QUOTE]

fclose(outfile) can be called with an uninitialized outfile in the
above. So besides protecting by detecting NULL value, you need to
initialized them to NULL.

As mentioned in the comment you quoted.

-- Richard

 

[Keith Thompson]

You are laboring under a dangerous misunderstanding here.

Here is what C90 says about atoi(), atol(), and atof():

"The functions atof, atoi, and atol need not affect the value of the
integer expression errno on an error. If the value of the result
cannot be represented, the behavior is undefined."

The difference in C99 is that the new function atoll() is included:

"The functions atof, atoi, atol, and atoll need not affect the value
of the integer expression errno on an error. If the value of the
result cannot be represented, the behavior is undefined."

The ato... functions all invoke undefined behavior if the text string
represents a value outside the range of the function's return type. No
implementation-defined behavior or implementation-defined signal, just
plain old UB.

Argh, how did I miss that?

Or, rather, arrrrrrrrrrgh (this being International Talk Like a Pirate
Day).

 

[CBFalconer]

The way the original poster described it, what you or I would
call "doing a proper job" is not what his boss calls "doing a
proper job". He would get blame and lose reputation much earlier,
and possibly his job as well.

While there may be exceptions [1] I expect that the 'boss' will
never notice, but both he and the programmer will get the
reputation of producing good code.

[1] Exceptions include such things as embedded packages needing
absolute minimum code space. If the boss ever notices he has to be
a reasonably capable programmer himself, and can probably
appreciate the necessity for those checks.

 

[CBFalconer]

(e-mail address removed) wrote:
.... snip ...


You are laboring under a dangerous misunderstanding here.

Here is what C90 says about atoi(), atol(), and atof():

"The functions atof, atoi, and atol need not affect the value
of the integer expression errno on an error. If the value of
the result cannot be represented, the behavior is undefined."

The difference in C99 is that the new function atoll() is
included:

"The functions atof, atoi, atol, and atoll need not affect the
value of the integer expression errno on an error. If the
value of the result cannot be represented, the behavior is
undefined."

The ato... functions all invoke undefined behavior if the text
string represents a value outside the range of the function's
return type. No implementation-defined behavior or
implementation-defined signal, just plain old UB.

I think you are missing the point. Those ato* functions are only
present to allow old code to be compiled. There is no purpose
whatsoever to the atoll code except to encourage foolishness. They
can all be replaced by strto* calls, which have proper error
reporting capabilities, and should be so replaced in any new code.

 

[Keith Thompson]

Jack Klein wrote: [...]

The ato... functions all invoke undefined behavior if the text
string represents a value outside the range of the function's
return type. No implementation-defined behavior or
implementation-defined signal, just plain old UB.

I think you are missing the point.

Huh? What makes you think that?

Those ato* functions are only
present to allow old code to be compiled. There is no purpose
whatsoever to the atoll code except to encourage foolishness. They
can all be replaced by strto* calls, which have proper error
reporting capabilities, and should be so replaced in any new code.

He didn't miss the point, he was providing an argument in favor of it.

 

[vippstar]

[ correcting me saying atoi doesn't invoke UB ]

You are laboring under a dangerous misunderstanding here.

Here is what C90 says about atoi(), atol(), and atof():

"The functions atof, atoi, and atol need not affect the value of the
integer expression errno on an error. If the value of the result
cannot be represented, the behavior is undefined."

The difference in C99 is that the new function atoll() is included:

"The functions atof, atoi, atol, and atoll need not affect the value
of the integer expression errno on an error. If the value of the
result cannot be represented, the behavior is undefined."

The ato... functions all invoke undefined behavior if the text string
represents a value outside the range of the function's return type. No
implementation-defined behavior or implementation-defined signal, just
plain old UB.

I see, thanks for this. Therefore atoi is certainly not equivalent to
(int)strtol(s, (char **)0, 10);
I couldn't see how it could be equivalent anyway, since this code is
valid

#include <stdlib.h>

....
#undef strtol
int i = atoi("42");

But it would break in an implementation where atoi is defined in terms
of strtol and strtol is a macro.

 

[Harald van Dijk]

I couldn't see how it could be equivalent anyway, since this code is
valid

#include <stdlib.h>

...
#undef strtol
int i = atoi("42");

But it would break in an implementation where atoi is defined in terms
of strtol and strtol is a macro.

How would it break? Even if a macro definition of strtol is provided, it
must also be available as a declaration of an actual function, just like
almost all other standard library functions.

 

[Keith Thompson]

I see, thanks for this. Therefore atoi is certainly not equivalent to
(int)strtol(s, (char **)0, 10);

No, nobody said it was equivalent. The standard clearly says it's
equivalent *except for the behavior on error*.

I couldn't see how it could be equivalent anyway, since this code is
valid

#include <stdlib.h>

...
#undef strtol
int i = atoi("42");

But it would break in an implementation where atoi is defined in terms
of strtol and strtol is a macro.

No, it wouldn't. C99 7.1.4p1:

Any function declared in a header may be additionally implemented
as a function-like macro defined in the header[...]. The use of
#undef to remove any macro definition will also ensure that an
actual function is referred to.