Style Question - Checking for Poor Parameters

Discussion in 'C Programming' started by Michael B Allen, Nov 25, 2003.

  1. 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
    Michael B Allen, Nov 25, 2003
    #1
    1. Advertising

  2. 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.
    E. Robert Tisdale, Nov 25, 2003
    #2
    1. Advertising

  3. 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 (Yes it's valid)
    +-----------------------------------------------------------+
    Linux: generous programmers from around the world all join
    forces to help you shoot yourself in the foot for free.
    Erik de Castro Lopo, Nov 25, 2003
    #3
  4. Michael B Allen

    Severian Guest

    On Tue, 25 Nov 2003 15:03:49 -0500, 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)
    >{
    > 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
    Severian, Nov 25, 2003
    #4
  5. Michael B Allen

    CBFalconer Guest

    "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 () ()
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net> USE worldnet address!
    CBFalconer, Nov 26, 2003
    #5
  6. Michael B Allen

    Severian Guest

    On Wed, 26 Nov 2003 05:00:16 GMT, CBFalconer <>
    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
    Severian, Nov 26, 2003
    #6
  7. 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
    Michael B Allen, Nov 26, 2003
    #7
  8. 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
    Michael B Allen, Nov 26, 2003
    #8
  9. Michael B Allen

    Severian Guest

    On Wed, 26 Nov 2003 04:17:27 -0500, Michael B Allen
    <> 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
    Severian, Nov 26, 2003
    #9
  10. Michael B Allen

    Rick Guest

    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
    Rick, Nov 26, 2003
    #10
  11. Michael B Allen

    Dan Pop Guest

    In <3fc4c867$0$1507$4all.nl> "Rick" <> writes:

    >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
    --
    Dan Pop
    DESY Zeuthen, RZ group
    Email:
    Dan Pop, Nov 26, 2003
    #11
  12. "Rick" <> wrote:

    [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
    --
    Irrwahn
    ()
    Irrwahn Grausewitz, Nov 26, 2003
    #12
  13. On Wed, 26 Nov 2003 04:49:15 -0500, Severian wrote:

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


    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
    Michael B Allen, Nov 26, 2003
    #13
  14. Irrwahn Grausewitz <> scribbled the following:
    > "Rick" <> wrote:
    > [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?

    --
    /-- Joona Palaste () ------------- Finland --------\
    \-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
    "'I' is the most beautiful word in the world."
    - John Nordberg
    Joona I Palaste, Nov 26, 2003
    #14
  15. Michael B Allen

    Alan Balmer Guest

    On 26 Nov 2003 20:58:49 GMT, Joona I Palaste <>
    wrote:

    >Irrwahn Grausewitz <> scribbled the following:
    >> "Rick" <> wrote:
    >> [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.

    --
    Al Balmer
    Balmer Consulting
    Alan Balmer, Nov 26, 2003
    #15
  16. Michael B Allen wrote:
    >
    > 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.


    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.

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


    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 (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!
    Erik de Castro Lopo, Nov 26, 2003
    #16
  17. Joona I Palaste <> wrote:

    > Irrwahn Grausewitz <> scribbled the following:
    > > "Rick" <> wrote:
    > >
    > >> 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.

    >
    > 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>
    --
    Irrwahn
    ()
    Irrwahn Grausewitz, Nov 26, 2003
    #17
  18. Michael B Allen

    Severian Guest

    On Wed, 26 Nov 2003 14:27:54 -0500, Michael B Allen
    <> wrote:

    >On Wed, 26 Nov 2003 04:49:15 -0500, Severian wrote:
    >
    >>>> 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.

    >
    >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
    Severian, Nov 26, 2003
    #18
  19. Michael B Allen <> wrote in message news:<>...
    > 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.
    Debashish Chakravarty, Nov 27, 2003
    #19
  20. On Wed, 26 Nov 2003 17:11:17 -0500, Severian wrote:

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


    Ahh. I see what you mean.

    Mike
    Michael B Allen, Nov 27, 2003
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?Utf-8?B?QmlsbCBUaG9ybmU=?=

    ASP.NET Poor performance with long-running COM object calls

    =?Utf-8?B?QmlsbCBUaG9ybmU=?=, Oct 12, 2004, in forum: ASP .Net
    Replies:
    4
    Views:
    646
    =?Utf-8?B?QmlsbCBUaG9ybmU=?=
    Nov 15, 2004
  2. Helge

    W3WP poor performance

    Helge, Mar 14, 2005, in forum: ASP .Net
    Replies:
    1
    Views:
    1,412
    Scott Allen
    Mar 14, 2005
  3. John Smith

    VB.Net bug or Poor Coding Practice?

    John Smith, Aug 11, 2005, in forum: ASP .Net
    Replies:
    3
    Views:
    471
    John Smith
    Aug 14, 2005
  4. Stephen Kershaw
    Replies:
    4
    Views:
    261
    Martin Ambuhl
    Mar 25, 2006
  5. Ken Varn
    Replies:
    0
    Views:
    428
    Ken Varn
    Apr 26, 2004
Loading...

Share This Page