Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Style Question - Checking for Poor Parameters

Reply
Thread Tools

Style Question - Checking for Poor Parameters

 
 
Michael B Allen
Guest
Posts: n/a
 
      11-25-2003
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
 
Reply With Quote
 
 
 
 
E. Robert Tisdale
Guest
Posts: n/a
 
      11-25-2003
Michael B Allen wrote:

> 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.

 
Reply With Quote
 
 
 
 
Erik de Castro Lopo
Guest
Posts: n/a
 
      11-25-2003
Michael B Allen wrote:
>
> 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 http://www.velocityreviews.com/forums/(E-Mail 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.
 
Reply With Quote
 
Severian
Guest
Posts: n/a
 
      11-25-2003
On Tue, 25 Nov 2003 15:03:49 -0500, Michael B Allen
<(E-Mail Removed)> wrote:

>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

 
Reply With Quote
 
CBFalconer
Guest
Posts: n/a
 
      11-26-2003
"E. Robert Tisdale" wrote:
> Michael B Allen wrote:
>
> > 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;

> }


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.

--
Chuck F ((E-Mail Removed)) ((E-Mail Removed))
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net> USE worldnet address!


 
Reply With Quote
 
Severian
Guest
Posts: n/a
 
      11-26-2003
On Wed, 26 Nov 2003 05:00:16 GMT, CBFalconer <(E-Mail Removed)>
wrote:

>"E. Robert Tisdale" wrote:
>> Michael B Allen wrote:
>>
>> > 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;

>> }

>
>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

 
Reply With Quote
 
Michael B Allen
Guest
Posts: n/a
 
      11-26-2003
On Tue, 25 Nov 2003 16:55:54 -0500, Erik de Castro Lopo wrote:
>
> 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
 
Reply With Quote
 
Michael B Allen
Guest
Posts: n/a
 
      11-26-2003
On Tue, 25 Nov 2003 17:28:45 -0500, Severian wrote:

> 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
 
Reply With Quote
 
Severian
Guest
Posts: n/a
 
      11-26-2003
On Wed, 26 Nov 2003 04:17:27 -0500, Michael B Allen
<(E-Mail Removed)> wrote:

>On Tue, 25 Nov 2003 17:28:45 -0500, Severian wrote:
>
>> 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.


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.

>> 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).


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.

>Mike


 
Reply With Quote
 
Rick
Guest
Posts: n/a
 
      11-26-2003
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


 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Poor, poor P&S owner learns too late... Rich Digital Photography 66 06-11-2009 04:48 AM
Number of Parameters -- A Question of Style and Form Hal Vaughan Java 4 06-15-2007 09:16 AM
question about checking the status of a style gerg Javascript 1 07-04-2006 08:47 PM
Poor reception, poor connection, and dropped signal =?Utf-8?B?dW51c3VhbHBzeWNobw==?= Wireless Networking 2 06-07-2006 12:54 AM
Need help with Style conversion from Style object to Style key/value collection. Ken Varn ASP .Net Building Controls 0 04-26-2004 07:06 PM



Advertisments