Welcome to MSDN Blogs Sign in | Join | Help

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.

Published Monday, October 02, 2006 11:50 AM by mgrier

Comments

# re: Canonical Function Structure

Sunday, October 08, 2006 5:24 AM by Bruno van Dooren

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.

# re: Canonical Function Structure

Friday, October 20, 2006 7:25 PM by JeffAbraham

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.

# re: Canonical Function Structure

Thursday, November 02, 2006 5:11 AM by Raul Igrisan

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] = ‘\0’;'

?

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

# re: Canonical Function Structure

Monday, November 06, 2006 11:59 PM by Norman Diamond

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.

# re: Canonical Function Structure

Friday, November 10, 2006 1:06 PM by Jon Wiswall

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.

New Comments to this post are disabled
 
Page view tracker