Canonical Function Structure


Here is a proposed canonical structure for functions/procedures.  Clearly in some cases some sections would not be present.  What’s important is to understand the basic phasing of the work to be done and the separation of tasks.


 


int foo(


     char *StringIn, // “in” parameter


     size_t BufferLength, // “in” parameter


     char Buffer[], // “out” parameter, max length governed by BufferLength


     size_t *CharsWritten // “out” parameter, must be <= BufferLength


     ) {


     // Phase 1: Initialize out parameters


     *CharsWritten = 0;


 


     // Phase 2: Capture and validate parameters


     const size_t StringInLength = strlen(StringIn);


     if ((BufferLength != 0) && (Buffer == NULL))


return ERR_INVALID_PARAMETER;


 


     // Phase 3: do work


if (BufferLength <= StringInLength)


     return ERR_BUFFER_TOO_SMALL;


     memcpy(Buffer, StringIn, StringInLength);


     Buffer[StringInLength] = ‘\0’;


 


     // Phase 4: copy data out


     *CharsWritten = StringInLength + 1; // safe because BufferLength is > StringInLength so “+1” won’t overflow


 


     // Phase 5: goodbye


     return ERR_SUCCEEDED;


}


 


There may be other phases and the example’s perhaps kind of dumb but I think it works well to call out what kinds of operations should happen in which phases and even more so what doesn’t have to happen later on given work that occurred earlier.


Comments (5)

  1. I have never thought about it formally, and you are right, but isn’t this kicking in an open door?

    I have read your articles on the dll loader. They are at expert level.

    This article is more of the ‘this is a compiler. it turns source code into executable code’ level.

    Is this perhaps the start of a new series of articles?

    Good to see that you are writing again.

  2. JeffAbraham says:

    Ignoring that this is a trivial example, I have a couple of questions about the style…

    Are you intentionally barfing on bad values of CharsWritten (i.e. null) to allow for early and obvious error detection?

    It looks like StringInLength validation is occurring in the "do work" stage, I think of that more as parameter validation.

    In my ideal world, the "do work" stage doesn’t do anything that can fail AND be observed by a caller of the function (as long as you are in a good state at the start of the call), i.e. some kind of transactional guarantee.

  3. Raul Igrisan says:

    Didn’t you mean ‘(BufferLength == 0)’ instead of ‘(BufferLength != 0)’ in Phase 2? (Or even < 1 since you’re planning to copy the terminal NULL).

    Wouldn’t be better to put the required buffer size in CharsWritten before returning ERR_BUFFER_TOO_SMALL? That way the caller may first want to check the required buffer size, than allocate a largely enough buffer and call again and expect to succeed.

    Wouldn’t

    ‘memcpy(Buffer, StringIn, StringInLength+1);’ have the same effect as

    ‘memcpy(Buffer, StringIn, StringInLength);

    Buffer[StringInLength] = ‘’;’

    ?

    I would also check source and destination buffers to not overlap.

  4. Norman Diamond says:

    To Raul Igrisan:

    if ((BufferLength != 0) && (Buffer == NULL))

    diagnoses an error when the caller pretends to have a nonzero length of memory at location NULL.  This test allows the caller to have no memory at all at location NULL.

    A later test diagnoses an error if BufferLength is actually zero.

    > I would also check source and destination

    > buffers to not overlap.

    I agree on this one.  Mr. Grier called memcpy() not memmove() so his code can yield undefined behaviour unless he adds this validation.

  5. Jon Wiswall says:

    Norman, Raul – that depends entirely on the documented contract for the function, which is lacking here; if that says "the input and output buffers may not overlap", then there you have it.

Skip to main content