Style Question - Checking for Poor Parameters

[Michael B Allen]

I have a general purpose library that has a lot of checks for bad input
parameters like:

void *
linkedlist_get(struct linkedlist *l, unsigned int idx)
{
if (l == NULL) {
errno = EINVAL;
return NULL;
}

if (idx >= l->size) {
errno = ERANGE;
return NULL;
}
if (idx == 0) {
return l->first->data;
...

Now in practice most of these instances would never be triggered unless
there was a flaw in the calling code in which case the program will fault
and the problem will be exposed during the testing phase. The question
is; does the user of a general purpose C library prefer these checks,
asserts, or no checks at all?

Mike

 

[E. Robert Tisdale]

I have a general purpose library
that has a lot of checks for bad input parameters like:

void *
linkedlist_get(struct linkedlist *l, unsigned int idx) {
void* result = NULL;
if (NULL != l) {
if (idx < l->size) {
if (0 == idx) {
result = l->first->data;
// . . .
}
}
else { // idx >= l->size
errno = ERANGE;
}
else { // NULL == l
errno = EINVAL;
}
return result;

}

Now, in practice,
most of these instances would never be triggered
unless there was a flaw (bug) in the calling code
in which case the program will fault
and the problem will be exposed during the testing phase.
The question is; does the user of a general purpose C library
prefer these checks, asserts, or no checks at all?

Use the assert C preprocessor macro
to detect programming errors (bugs).
Use the NDEBUG C preprocessor macro to turn off assert
after you have completed testing and debugging.

Use the checks above to detect *exceptions*.
(I wouldn't use errno. I'd return an exception object.)
Exceptions are expected but unpredictable events
which cannot be prevented but must be "handled"
either at the point where they are first detected
or in the calling program.
The exception object must contain *all* of the information
required to handle the exception in the calling program.

 

[Erik de Castro Lopo]

Now in practice most of these instances would never be triggered unless
there was a flaw in the calling code
Agreed.

in which case the program will fault
and the problem will be exposed during the testing phase.

Just look at all the biggy programs out there and you can see
that this statement is false .

"Testing can prove the presence of bugs, but never their absence."
-- Edsger Dijkstra

The question
is; does the user of a general purpose C library prefer these checks,
asserts, or no checks at all?

You are doing the right thing.

No checking is bad because the program will continue running
with bad state and crash somewhere else. The later crash may
or may not have any relation to where the bug is. If it is
unrelated it will be very difficult to figure out where the
real bug is.

In addition, if your library is going to be used by other
people and it crashes in your code, you will be blamed for
the crash even though the other person passed in bad data.

Asserts are bad because program just prints an error message
and exits the program. This prevents the calling code from
attempting some sort of corrective action.

Erik
--
+-----------------------------------------------------------+
Erik de Castro Lopo (e-mail address removed) (Yes it's valid)
+-----------------------------------------------------------+
Linux: generous programmers from around the world all join
forces to help you shoot yourself in the foot for free.

 

[Severian]

I have a general purpose library that has a lot of checks for bad input
parameters like:

void *
linkedlist_get(struct linkedlist *l, unsigned int idx)
{
if (l == NULL) {
errno = EINVAL;
return NULL;
}

if (idx >= l->size) {
errno = ERANGE;
return NULL;
}
if (idx == 0) {
return l->first->data;
...

Now in practice most of these instances would never be triggered unless
there was a flaw in the calling code in which case the program will fault
and the problem will be exposed during the testing phase. The question
is; does the user of a general purpose C library prefer these checks,
asserts, or no checks at all?

As someone who has used and written quite a few libraries, I've found
it most helpful for them to return a status code which can be tested,
and to provide a function to convert the status code to a string.

When it's appropriate for a lot of related functions to return
pointers, providing a library_geterror() routine is an option as well.

I prefer libraries not use errno (except of course the standard
library). Most libraries need to report errors other than the C
standard errors. Also, it's often useful to call standard library
functions during error logging or reporting, and having to shuffle
errno is a PITA.

In complex libraries, especially those dealing with unpredicatable
external data, I like being able to set error and warning callbacks as
well.

I wouldn't recommend assert() for *external* sanity checks if other
people will be using your library. It's fine for internal stuff,
though.

- Sev

 

[CBFalconer]

}

No he didn't write that. Not only are you a consumate idiot in
the C advice you give, you are systematically misstating what
others wrote for your own trollish purposes.

This systematic altering of someone elses words is far worse, in
my mind, than any other sins Trollsdale has committed here. The
others can be put down to stupidity, ignorance, and poor
upbringing, but this goes far beyond the pale.

 

[Severian]

No he didn't write that. Not only are you a consumate idiot in
the C advice you give, you are systematically misstating what
others wrote for your own trollish purposes.

This systematic altering of someone elses words is far worse, in
my mind, than any other sins Trollsdale has committed here. The
others can be put down to stupidity, ignorance, and poor
upbringing, but this goes far beyond the pale.

I briefly wondered why it looked different, but I didn't actually
reread the code. I assumed it was just a newsreader-formatting-flub.

ERT wouldn't last a week at my company. What horrid code! What a tard!

- Sev

 

[Michael B Allen]

No checking is bad because the program will continue running with bad
state and crash somewhere else. The later crash may or may not have any
relation to where the bug is. If it is unrelated it will be very
difficult to figure out where the real bug is.

Not necessarily. Dereferecing NULL is going to fault on the spot.

In addition, if your library is going to be used by other people and it
crashes in your code, you will be blamed for the crash even though the
other person passed in bad data.

Not necessarily There are quite a few well known or standard library
functions that expect suitable inputs.

Asserts are bad because program just prints an error message and exits
the program. This prevents the calling code from attempting some sort of
corrective action.

Agreed. It's hard to ship code with asserts. Certainly not mature code.

Well I think I'm going to stick with leaving the checks as it is obvious
there is no consensus on this issue.

Mike

 

[Michael B Allen]

As someone who has used and written quite a few libraries, I've found it
most helpful for them to return a status code which can be tested, and
to provide a function to convert the status code to a string.

I have macros and some functions for doing just that. I just don't do
it from trivial things like the linkedlist example shown.

When it's appropriate for a lot of related functions to return pointers,
providing a library_geterror() routine is an option as well.

I prefer libraries not use errno (except of course the standard
library). Most libraries need to report errors other than the C standard
errors. Also, it's often useful to call standard library functions
during error logging or reporting, and having to shuffle errno is a
PITA.

For non-trivial stuff yes. If for no other reason that errno clashes
with reentrant code (which I think is basically what you just said). For
trivial stuff like a linkedlist ADT it's hard to justify adding an error
code member to struct linkedlist.

Mike

 

[Severian]

I have macros and some functions for doing just that. I just don't do
it from trivial things like the linkedlist example shown.

I do too, and write them specifically for imported libraries when
needed. Like you, I don't use others' libraries for trivial things
like linked lists; but I understood the question as applying to much
more general (and more widely distributed) libraries.

For non-trivial stuff yes. If for no other reason that errno clashes
with reentrant code (which I think is basically what you just said).

Yes, reentrancy (and threads) are things I have to deal with. But I
was simply thinking of a library returning an error that I need to
report: the reporting process may require me to call standard library
functions before using errno, which could overwrite errno. So, even in
the absence of reentrancy or threads, I would have to remember to copy
its value to another variable. Not a big deal, but something not
necessary if the library routines return their own status codes.

For
trivial stuff like a linkedlist ADT it's hard to justify adding an error
code member to struct linkedlist.

Absolutely! I wouldn't think of using someone else's library for
something as trivial as that, and I would code my one quite
differently. Again, I viewed his sample code as showing some things a
"general purpose" library routine could check for. I suppose I read
that as something that lots of other people would be using.

 

[Rick]

Stop fighting kids. I'm sure you're all talented programmers but the way how
some people deal with each other is horrible. Get off that computer and
learn some basic social stuff first, that's what makes us differ from
animals.

Greetinsg,
Rick

 

[Dan Pop]

Stop fighting kids. I'm sure you're all talented programmers but the way how ^^^^
some people deal with each other is horrible. Get off that computer and
learn some basic social stuff first, that's what makes us differ from
animals.

You may want to follow your own advice.

Dan

 

[Irrwahn Grausewitz]

[Please keep some context, including attributions, when replying; it
makes it much easier for others to follow discussions. Thank you.]

Stop fighting kids. I'm sure you're all talented programmers but the way how
some people deal with each other is horrible. Get off that computer and
learn some basic social stuff first, that's what makes us differ from
animals.

Neither of them is a kid. And E.R.T. is a well known troll who
regularly alters quoted text without indication, in order to mess
up usenet discussions. That's not just a Bad Thing[tm], it's
abusive behaviour.

My 0.02 EUR.

Regards

 

[Michael B Allen]

Yes, reentrancy (and threads) are things I have to deal with. But I was
simply thinking of a library returning an error that I need to report:
the reporting process may require me to call standard library functions
before using errno, which could overwrite errno. So, even in the absence
of reentrancy or threads, I would have to remember to copy its value to
another variable.

I don't really follow. Consider setlocale. It has a tendency to set
errno as it searchs and fails to find various locale files even though
it ultimately returns success. If I call that, it sets errno, and I do
something else that can set errno then at no point do I need to save
errno as you describe. Either I return success in which case the caller
should happily proceed or I return -1 indicating errno should be examined
at which point it should be what I set it to when the error occured
(e.g. EIO).

int
my_fn(char *l)
{
setlocale(LC_ALL, l);

if (another_fn() == -1) {
errno = EIO;
return -1;
}

return 0;
}

Otherwise, it is very poor design IMO to require checking errno to
determine if an error occured. Occationally it is unreasonable to
do otherwise but in this case an error member specific to the object
associated with the error should be used with an additional function to
retrieve it (e.g. fread).

Can you be more specific about where you would be compelled to "shuffle
errno"?

Mike

 

[Joona I Palaste]

[Please keep some context, including attributions, when replying; it
makes it much easier for others to follow discussions. Thank you.]

Stop fighting kids. I'm sure you're all talented programmers but the way how
some people deal with each other is horrible. Get off that computer and
learn some basic social stuff first, that's what makes us differ from
animals.

Neither of them is a kid. And E.R.T. is a well known troll who
regularly alters quoted text without indication, in order to mess
up usenet discussions. That's not just a Bad Thing[tm], it's
abusive behaviour.

My 0.02 EUR.

But he didn't call them kids. He asked them to stop fighting kids. I
think that's wise. After all, the kids are hardly a match for them,
are they?

 

[Alan Balmer]

[Please keep some context, including attributions, when replying; it
makes it much easier for others to follow discussions. Thank you.]

Stop fighting kids. I'm sure you're all talented programmers but the way how
some people deal with each other is horrible. Get off that computer and
learn some basic social stuff first, that's what makes us differ from
animals.

Neither of them is a kid. And E.R.T. is a well known troll who
regularly alters quoted text without indication, in order to mess
up usenet discussions. That's not just a Bad Thing[tm], it's
abusive behaviour.

My 0.02 EUR.

But he didn't call them kids. He asked them to stop fighting kids. I
think that's wise. After all, the kids are hardly a match for them,
are they?

But talented programmers have to do *something* for relaxation! You
have to admit that fighting kids is basic social stuff, after all, and
it does get one off the computer. I'm not so sure it differentiates us
from animals, though. Most of us actually are animals, though there
may be some vegetative tendencies here and there.

 

[Erik de Castro Lopo]

Not necessarily. Dereferecing NULL is going to fault on the spot.

But what if the NULL pointer is passed into the library which
stores it for use later. It is this later use of an unvalidated
pointer which will cause the segfault and then you need to
figure out how the NULL pointer got to its present state.

Not necessarily There are quite a few well known or standard library
functions that expect suitable inputs.

As an author of two widely used Free Software libraries I can
assure you that it you remove the possibility of your software
being blamed you will have a much easier time of supporting
it .

Erik
--
+-----------------------------------------------------------+
Erik de Castro Lopo (e-mail address removed) (Yes it's valid)
+-----------------------------------------------------------+
Peter F. Curran : Microsoft. Still delivering a text editor with
Win95/98 that can only open a max 64K file, despite being on
a machine with an 8Gig HD and 64M of ram....
G Cook: Perhaps, but Notepad is still the most functional program
in the whole suite!

 

[Irrwahn Grausewitz]

Neither of them is a kid. And E.R.T. is a well known troll who
regularly alters quoted text without indication, in order to mess
up usenet discussions. That's not just a Bad Thing[tm], it's
abusive behaviour.

But he didn't call them kids. He asked them to stop fighting kids. I
think that's wise. After all, the kids are hardly a match for them,
are they?

I don't know about his strength, though. However, maybe he had
only one comma left and thought it would look better in the second
sentence... <g,d&r>

 

[Severian]

I don't really follow. Consider setlocale. It has a tendency to set
errno as it searchs and fails to find various locale files even though
it ultimately returns success. If I call that, it sets errno, and I do
something else that can set errno then at no point do I need to save
errno as you describe. Either I return success in which case the caller
should happily proceed or I return -1 indicating errno should be examined
at which point it should be what I set it to when the error occured
(e.g. EIO).

int
my_fn(char *l)
{
setlocale(LC_ALL, l);

if (another_fn() == -1) {
errno = EIO;
return -1;
}

return 0;
}

Otherwise, it is very poor design IMO to require checking errno to
determine if an error occured. Occationally it is unreasonable to
do otherwise but in this case an error member specific to the object
associated with the error should be used with an additional function to
retrieve it (e.g. fread).

Can you be more specific about where you would be compelled to "shuffle
errno"?

It's pretty contrived, but this should show what I mean:

if (my_fun("somestring") < 0) {
FILE *flog = fopen("error.log", "a+"); /* errno may be gone! */
if (flog) {
fprintf(flog, "my_fun reports: %d\n", errno);
fclose(flog);
}
return FALSE;
}

- Sev


- Sev

 

[Debashish Chakravarty]

I have a general purpose library that has a lot of checks for bad input
parameters like:

[snip]

The only problem I can see is if one of your functions calls another
one of your functions and both of them check for bad input, so you
might be testing for the same kind of invalid input twice.

 

[Michael B Allen]

It's pretty contrived, but this should show what I mean:

if (my_fun("somestring") < 0) {
FILE *flog = fopen("error.log", "a+"); /* errno may be gone! */ if
(flog) {
fprintf(flog, "my_fun reports: %d\n", errno); fclose(flog);
}
return FALSE;
}

Ahh. I see what you mean.

Mike