James's Weblog

People writing C code often write functions that look like:

/** foo -- Some function.
  *
  * PARAMETERS:
  *   someInput       : Some input value.
  *   OUT someOutputP : Set to some output value.
  *
  * RETURNS:
  *   Some error code.
  */
err_t foo(int someInput, data_t* someOutputP)
{
    err_t err = ERR_NONE;
    /* ... Code goes here ... */
    return err;
}

This isn’t good enough. Callers have no idea what to expect if an error occurs. Is someOutputP:

• in some totally undefined state?
• set to a known, bogus value?
• left untouched?

This is particularly important if the function allocates some resources in someOutputP and the caller needs to clean it up.

Whichever method you choose, you should choose one and document it. It needs to be part of the function’s contract. If left undocumented, callers can make assumptions about what state someOutputP is in, which is dangerous.

Worse, if you don’t specify the behavior, someone who modifies the code later might not know what the behavior is supposed to be and is likely to screw it up.

Better:

/** foo -- Some function.
  *
  * PARAMETERS:
  *   someInput       : Some input value.
  *   OUT someOutputP : On success, set to some output value.
  *                     On error, left untouched.
  *
  * RETURNS:
  *   Some error code.
  */
err_t foo(int someInput, data_t* someOutputP)
{
    err_t err = ERR_NONE;
    /* ... Code goes here ... */
    return err;
}

I’ll write my thoughts about the three behaviors later.