"Automatic" Resource Cleanup?

[Shao Miller]

Where one uses macros instead of jump statements (though the actual
statements _could_ be overridden, theoretically) and where one uses the
'HAS_CLEANUP' macro within whatever scopes require resource cleanup.

Also available (with nice colours) at:

http://codepad.org/YLV13z1K

Can be compiled all as one file via copy and paste, or split into
component files.

Feedback welcome and appreciated, as always.

-----

/*
* To split into separate files, remove all
* lines containing 'ALL_IN_ONE'
*/
#define ALL_IN_ONE 1

/****
* cleanup.h
* 2011, Shao Miller
*/
#ifndef CLEANUP_H_
#include <stddef.h>

/*** Macros */
#define CLEANUP_H_

/**
* Block cleanup details
*/
#define CLEANUP_BLOCK_ cleanup_do_(cleanup_);

/**
* Function cleanup details
*/
#define CLEANUP_FUNC_ cleanup_chain_(cleanup_);

/**
* Perform block cleanup and jump to a label.
* Only use for jumping from an inner to an
* outer scope.
* Note the trailing space after 'goto'.
*/
#define c_goto \
switch (1) \
while (1) \
if (0) { \
default: { \
CLEANUP_BLOCK_; \
} \
} else \
goto

/**
* Perform block cleanup and continue a loop
*/
#define c_continue \
if (1) { \
CLEANUP_BLOCK_; \
continue; \
} else do ; while (0)

/**
* Perform block cleanup and break out of a
* a loop or 'switch' statement
*/
#define c_break \
if (1) { \
CLEANUP_BLOCK_; \
break; \
} else do ; while (0)

/**
* Perform function cleanup and return from
* a function.
* Note the trailing space after 'goto'
*/
#define c_return \
switch (1) \
while (1) \
if (0) { \
default: { \
CLEANUP_FUNC_; \
} \
} else \
return

/**
* An integer constant expression which
* declares an enumeration value that is one
* greater than the same-name enumeration
* value in the containing scope. This
* is for internal use; don't use it
*/
#define CV_CLEANUP_ ((enum { \
cv_cleanup_ = cv_cleanup_ + 1 \
})1)

/**
* This macro establishes cleanup context for
* a function or compound statement. Use it
* only where declarations are allowed, such as
* at the beginning of a function or compound
* statement in C89.
*/
#define HAS_CLEANUP \
cleanup_t_ \
new_cleanup_[CV_CLEANUP_] = {{0}}, \
* p_new_cleanup_ = cleanup_init_( \
new_cleanup_, \
cleanup_, \
cv_cleanup_ \
), \
* cleanup_ = p_new_cleanup_

#ifndef ALL_IN_ONE
/* Wrap malloc */
#ifdef malloc
#undef malloc
#endif
#define malloc(size) c_malloc_(cleanup_, (size))
#endif

/*** Constants */
enum { cv_cleanup_ };

/*** Object types */

/* Internal use; don't use it */
struct s_cleanup_ {
struct s_cleanup_ * prev;
void * item;
};
typedef struct s_cleanup_ cleanup_t_;

/*** Objects */

/**
* Top-level cleanup context (empty).
* Internal use; don't use it
*/
extern cleanup_t_ cleanup_[1];

/*** Functions */

/* Internal use; don't use these */
extern cleanup_t_ * cleanup_init_(
cleanup_t_ *,
cleanup_t_ *,
int
);
extern void * c_malloc_(cleanup_t_ *, size_t);
extern void cleanup_do_(cleanup_t_ *);
extern void cleanup_chain_(cleanup_t_ *);

#endif /* CLEANUP_H_ */


/****
* cleanup.c
* 2011, Shao Miller
*/
#include <stdlib.h>
#include <stdio.h>
#ifndef ALL_IN_ONE
#include "cleanup.h"
#endif /* ALL_IN_ONE */

/*** Object types */

/* A cleanable item */
struct s_cleanup_item_ {
void * ref;
struct s_cleanup_item_ * next;
};
typedef struct s_cleanup_item_ s_cleanup_item;

/*** Objects */
cleanup_t_ cleanup_[1] = {{0, 0}};

/*** Functions */
cleanup_t_ * cleanup_init_(
cleanup_t_ * new,
cleanup_t_ * prev,
int level
) {
new->prev = prev;
return new;
}

void * c_malloc_(
cleanup_t_ * cleanup,
size_t size
) {
s_cleanup_item * item;
void * obj;

item = (malloc)(sizeof *item);
if (!item)
return item;

obj = (malloc)(size);
if (!obj) {
free(item);
return obj;
}

item->ref = obj;
/* Insert at beginning */
item->next = cleanup->item;
cleanup->item = item;
return obj;
}

static char * at_root(cleanup_t_ * cleanup) {
if (cleanup == cleanup_)
return " (root)";
return "";
}

void cleanup_do_(cleanup_t_ * cleanup) {
s_cleanup_item * item, * next;

printf(
"Cleanup @ %p%s\n",
(void *) cleanup,
at_root(cleanup)
);
item = cleanup->item;
cleanup->item = 0;
while (item) {
next = item->next;
printf("Freeing object @ %p\n", item->ref);
free(item->ref);
free(item);
item = next;
}
return;
}

void cleanup_chain_(cleanup_t_ * cleanup) {
printf(
"Cleaning chain @ %p%s\n",
(void *) cleanup,
at_root(cleanup)
);
while (cleanup) {
cleanup_t_ * next;

cleanup_do_(cleanup);
next = cleanup->prev;
cleanup->prev = 0;
cleanup = next;
}
return;
}


/**** Test program */

#ifndef ALL_IN_ONE
#include <stdio.h>
#include "cleanup.h"
#endif /* ALL_IN_ONE */

#ifndef malloc
/* Wrap malloc the same as cleanup.h does */
#ifdef malloc
#undef malloc
#endif
#define malloc(size) c_malloc_(cleanup_, (size))
#endif

/*** Functions */

/* Dummy */
int true_func(int x) { return x + 1; }

void test1_c_goto(void) {
puts("\ntest1_c_goto():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
/* Used in an 'if'-'else' */
if (true_func(0))
c_goto outer_scope;
else
puts("Failed!");
return;
}
outer_scope:
puts("Exiting");
return;
}

void test2_c_goto(void) {
puts("\ntest2_c_goto():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
c_goto outer_scope;
puts("Failed!");
return;
}
outer_scope:
puts("Exiting");
return;
}

void test3_c_continue(void) {
/* Expecting: 4 loops */
int x = 5;

puts("\ntest3_c_continue():");
while (--x) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
if (true_func(0))
c_continue;
else
puts("Oops!");
puts("Failed!");
return;
}
puts("Exiting");
return;
}

void test4_c_break(void) {
/* Expecting: 1 loop */
int x = 5;

puts("\ntest4_c_break():");
while (--x) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
if (true_func(0))
c_break;
else
puts("Oops!");
puts("Failed!");
return;
}
puts("Exiting");
return;
}

/** Function "returning 'void'" */
void test5_c_return(void) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

puts("\ntest5_c_return():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip3 = malloc(sizeof *ip3);
int * ip4 = malloc(sizeof *ip4);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
printf("ip3: %p\n", (void *) ip3);
printf("ip4: %p\n", (void *) ip4);
if (true_func(0)) {
puts("Attempting c_return");
c_return;
} else
puts("Oops!");
}
puts("Failed!");
return;
}

/** Function returning 'int' */
int test6_c_return(void) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

puts("\ntest6_c_return():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip3 = malloc(sizeof *ip3);
int * ip4 = malloc(sizeof *ip4);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
printf("ip3: %p\n", (void *) ip3);
printf("ip4: %p\n", (void *) ip4);
if (true_func(0)) {
puts("Attempting c_return");
c_return 1;
} else
puts("Oops!");
}
puts("Failed!");
return 0;
}

/** Function returning 'int' */
int test7_c_return(void) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

puts("\ntest7_c_return():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip3 = malloc(sizeof *ip3);
int * ip4 = malloc(sizeof *ip4);

/* Another inner scope, below */
{
HAS_CLEANUP;
int * ip5 = malloc(sizeof *ip5);
int * ip6 = malloc(sizeof *ip6);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
printf("ip3: %p\n", (void *) ip3);
printf("ip4: %p\n", (void *) ip4);
printf("ip5: %p\n", (void *) ip5);
printf("ip6: %p\n", (void *) ip6);
if (true_func(0)) {
puts("Attempting c_return");
c_return 1;
} else
puts("Oops!");
}
}
puts("Failed!");
return 0;
}

/** Program entry-point */
int main(void) {
test1_c_goto();
test2_c_goto();
test3_c_continue();
test4_c_break();
test5_c_return();
if (!test6_c_return())
puts("Failed!");
if (!test7_c_return())
puts("Failed!");
return 0;
}

 

[Ian Collins]

Where one uses macros instead of jump statements (though the actual
statements _could_ be overridden, theoretically) and where one uses the
'HAS_CLEANUP' macro within whatever scopes require resource cleanup.

Also available (with nice colours) at:

http://codepad.org/YLV13z1K

Can be compiled all as one file via copy and paste, or split into
component files.

Feedback welcome and appreciated, as always.

I was just going to say "if you want automatic cleanup, use C++"! But I
see you have put a lot of effort into this, so I'll have a play..

 

[James Waldby]

Where one uses macros instead of jump statements (though the actual
statements _could_ be overridden, theoretically) and where one uses the
'HAS_CLEANUP' macro within whatever scopes require resource cleanup.

That is a sentence fragment (needs subject and verb) and you
probably should have a paragraph or two of comments in the code,
telling what problem it solves and why or when to use it.

....

http://codepad.org/YLV13z1K ....
/**
* Perform function cleanup and return from * a function.
* Note the trailing space after 'goto' */
#define c_return \
switch (1) \
while (1) \
if (0) { \
default: { \
CLEANUP_FUNC_; \
} \
} else \
return

....

'goto' looks like a typo in line 65 comment.

 

[Shao Miller]

That is a sentence fragment (needs subject and verb) and you
probably should have a paragraph or two of comments in the code,
telling what problem it solves and why or when to use it.

Ah yes. The previous post might've been better off beginning with an
ellipsis to suggest a continuation of the subject line (which, in turn,
might've been better off ending with an ellipsis.)

At this time, I'll simply offer that it's an example for how one might
"forget" about calling 'free' for objects allocated with 'malloc' within
functions and compound statements.

...
...

'goto' looks like a typo in line 65 comment.

Excellent catch; thanks. Yes, 'twas a copy'n'paste error. Fixed (along
with some missing 'ALL_IN_ONE' text):

http://codepad.org/LthMY2Il

-----

/*
* To split into separate files, remove all
* lines containing 'ALL_IN_ONE'
*/
#define ALL_IN_ONE 1

/****
* cleanup.h
* 2011, Shao Miller
*/
#ifndef CLEANUP_H_
#include <stddef.h>

/*** Macros */
#define CLEANUP_H_

/**
* Block cleanup details
*/
#define CLEANUP_BLOCK_ cleanup_do_(cleanup_);

/**
* Function cleanup details
*/
#define CLEANUP_FUNC_ cleanup_chain_(cleanup_);

/**
* Perform block cleanup and jump to a label.
* Only use for jumping from an inner to an
* outer scope.
* Note the trailing space after 'goto'.
*/
#define c_goto \
switch (1) \
while (1) \
if (0) { \
default: { \
CLEANUP_BLOCK_; \
} \
} else \
goto

/**
* Perform block cleanup and continue a loop
*/
#define c_continue \
if (1) { \
CLEANUP_BLOCK_; \
continue; \
} else do ; while (0)

/**
* Perform block cleanup and break out of a
* a loop or 'switch' statement
*/
#define c_break \
if (1) { \
CLEANUP_BLOCK_; \
break; \
} else do ; while (0)

/**
* Perform function cleanup and return from
* a function.
* Note the trailing space after 'return'
*/
#define c_return \
switch (1) \
while (1) \
if (0) { \
default: { \
CLEANUP_FUNC_; \
} \
} else \
return

/**
* An integer constant expression which
* declares an enumeration value that is one
* greater than the same-name enumeration
* value in the containing scope. This
* is for internal use; don't use it
*/
#define CV_CLEANUP_ ((enum { \
cv_cleanup_ = cv_cleanup_ + 1 \
})1)

/**
* This macro establishes cleanup context for
* a function or compound statement. Use it
* only where declarations are allowed, such as
* at the beginning of a function or compound
* statement in C89.
*/
#define HAS_CLEANUP \
cleanup_t_ \
new_cleanup_[CV_CLEANUP_] = {{0}}, \
* p_new_cleanup_ = cleanup_init_( \
new_cleanup_, \
cleanup_, \
cv_cleanup_ \
), \
* cleanup_ = p_new_cleanup_

#ifndef ALL_IN_ONE
/* Wrap malloc */
#ifdef malloc
#undef malloc
#endif
#define malloc(size) c_malloc_(cleanup_, (size))
#endif /* ALL_IN_ONE */

/*** Constants */
enum { cv_cleanup_ };

/*** Object types */

/* Internal use; don't use it */
struct s_cleanup_ {
struct s_cleanup_ * prev;
void * item;
};
typedef struct s_cleanup_ cleanup_t_;

/*** Objects */

/**
* Top-level cleanup context (empty).
* Internal use; don't use it
*/
extern cleanup_t_ cleanup_[1];

/*** Functions */

/* Internal use; don't use these */
extern cleanup_t_ * cleanup_init_(
cleanup_t_ *,
cleanup_t_ *,
int
);
extern void * c_malloc_(cleanup_t_ *, size_t);
extern void cleanup_do_(cleanup_t_ *);
extern void cleanup_chain_(cleanup_t_ *);

#endif /* CLEANUP_H_ */


/****
* cleanup.c
* 2011, Shao Miller
*/
#include <stdlib.h>
#include <stdio.h>
#ifndef ALL_IN_ONE
#include "cleanup.h"
#endif /* ALL_IN_ONE */

/*** Object types */

/* A cleanable item */
struct s_cleanup_item_ {
void * ref;
struct s_cleanup_item_ * next;
};
typedef struct s_cleanup_item_ s_cleanup_item;

/*** Objects */
cleanup_t_ cleanup_[1] = {{0, 0}};

/*** Functions */
cleanup_t_ * cleanup_init_(
cleanup_t_ * new,
cleanup_t_ * prev,
int level
) {
new->prev = prev;
return new;
}

void * c_malloc_(
cleanup_t_ * cleanup,
size_t size
) {
s_cleanup_item * item;
void * obj;

item = (malloc)(sizeof *item);
if (!item)
return item;

obj = (malloc)(size);
if (!obj) {
free(item);
return obj;
}

item->ref = obj;
/* Insert at beginning */
item->next = cleanup->item;
cleanup->item = item;
return obj;
}

static char * at_root(cleanup_t_ * cleanup) {
if (cleanup == cleanup_)
return " (root)";
return "";
}

void cleanup_do_(cleanup_t_ * cleanup) {
s_cleanup_item * item, * next;

printf(
"Cleanup @ %p%s\n",
(void *) cleanup,
at_root(cleanup)
);
item = cleanup->item;
cleanup->item = 0;
while (item) {
next = item->next;
printf("Freeing object @ %p\n", item->ref);
free(item->ref);
free(item);
item = next;
}
return;
}

void cleanup_chain_(cleanup_t_ * cleanup) {
printf(
"Cleaning chain @ %p%s\n",
(void *) cleanup,
at_root(cleanup)
);
while (cleanup) {
cleanup_t_ * next;

cleanup_do_(cleanup);
next = cleanup->prev;
cleanup->prev = 0;
cleanup = next;
}
return;
}


/**** Test program */

#ifndef ALL_IN_ONE
#include <stdio.h>
#include "cleanup.h"
#endif /* ALL_IN_ONE */

#ifndef malloc
/* Wrap malloc the same as cleanup.h does */
#ifdef malloc
#undef malloc
#endif
#define malloc(size) c_malloc_(cleanup_, (size))
#endif

/*** Functions */

/* Dummy */
int true_func(int x) { return x + 1; }

void test1_c_goto(void) {
puts("\ntest1_c_goto():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
/* Used in an 'if'-'else' */
if (true_func(0))
c_goto outer_scope;
else
puts("Failed!");
return;
}
outer_scope:
puts("Exiting");
return;
}

void test2_c_goto(void) {
puts("\ntest2_c_goto():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
c_goto outer_scope;
puts("Failed!");
return;
}
outer_scope:
puts("Exiting");
return;
}

void test3_c_continue(void) {
/* Expecting: 4 loops */
int x = 5;

puts("\ntest3_c_continue():");
while (--x) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
if (true_func(0))
c_continue;
else
puts("Oops!");
puts("Failed!");
return;
}
puts("Exiting");
return;
}

void test4_c_break(void) {
/* Expecting: 1 loop */
int x = 5;

puts("\ntest4_c_break():");
while (--x) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
if (true_func(0))
c_break;
else
puts("Oops!");
puts("Failed!");
return;
}
puts("Exiting");
return;
}

/** Function "returning 'void'" */
void test5_c_return(void) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

puts("\ntest5_c_return():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip3 = malloc(sizeof *ip3);
int * ip4 = malloc(sizeof *ip4);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
printf("ip3: %p\n", (void *) ip3);
printf("ip4: %p\n", (void *) ip4);
if (true_func(0)) {
puts("Attempting c_return");
c_return;
} else
puts("Oops!");
}
puts("Failed!");
return;
}

/** Function returning 'int' */
int test6_c_return(void) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

puts("\ntest6_c_return():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip3 = malloc(sizeof *ip3);
int * ip4 = malloc(sizeof *ip4);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
printf("ip3: %p\n", (void *) ip3);
printf("ip4: %p\n", (void *) ip4);
if (true_func(0)) {
puts("Attempting c_return");
c_return 1;
} else
puts("Oops!");
}
puts("Failed!");
return 0;
}

/** Function returning 'int' */
int test7_c_return(void) {
HAS_CLEANUP;
int * ip1 = malloc(sizeof *ip1);
int * ip2 = malloc(sizeof *ip2);

puts("\ntest7_c_return():");
/* Inner scope, below */
{
HAS_CLEANUP;
int * ip3 = malloc(sizeof *ip3);
int * ip4 = malloc(sizeof *ip4);

/* Another inner scope, below */
{
HAS_CLEANUP;
int * ip5 = malloc(sizeof *ip5);
int * ip6 = malloc(sizeof *ip6);

printf("ip1: %p\n", (void *) ip1);
printf("ip2: %p\n", (void *) ip2);
printf("ip3: %p\n", (void *) ip3);
printf("ip4: %p\n", (void *) ip4);
printf("ip5: %p\n", (void *) ip5);
printf("ip6: %p\n", (void *) ip6);
if (true_func(0)) {
puts("Attempting c_return");
c_return 1;
} else
puts("Oops!");
}
}
puts("Failed!");
return 0;
}

/** Program entry-point */
int main(void) {
test1_c_goto();
test2_c_goto();
test3_c_continue();
test4_c_break();
test5_c_return();
if (!test6_c_return())
puts("D'oh!");
if (!test7_c_return())
puts("D'oh!");
return 0;
}

 

[luser- -droog]

Where one uses macros instead of jump statements (though the actual
statements _could_ be overridden, theoretically) and where one uses the
'HAS_CLEANUP' macro within whatever scopes require resource cleanup.

Also available (with nice colours) at:

   http://codepad.org/YLV13z1K

Can be compiled all as one file via copy and paste, or split into
component files.

Feedback welcome and appreciated, as always.

<snip>

The spam almost made me miss this!
This is some scary stuff!

 

[Shao Miller]

<snip>

The spam almost made me miss this!
This is some scary stuff!

It could be adapted to reference-counting rather than straight 'free',
too... Perhaps using your reference-counting, pointer-hash-based code?

 

[Ian Collins]

<snip>

The spam almost made me miss this!

Spam? What spam?

 

[Shao Miller]

Spam? What spam?

Google Groups shows a bunch of spam:

https://groups.google.com/forum/#!forum/comp.lang.c

It's amazing that spam continues to generate revenue, after all these
years. *sigh*

 

[Tim Rentsch]

Where one uses macros instead of jump statements (though the actual
statements _could_ be overridden, theoretically) and where one uses
the 'HAS_CLEANUP' macro within whatever scopes require resource
cleanup.

Feedback welcome and appreciated, as always.
[snip posted source]

It would be TREMENDOUSLY helpful to take a different approach to
documentation style. For example,

/*
Several prose paragraphs, giving problem statement,
basic approach, perhaps a short list of primary
functions or other elements.
*/


#if 0 /* Example uses */

... some simple but illustrative examples ...

#endif


/* Description of interface elements, entry points:

name1 discussion/explanation

name2 discussion/explanation

... other macro names, type names, function names, etc ...

*/

... declarations (without contents) of public struct types ...
... public function prototypes ...


... now give code with "as few comments as necessary" ...
... (in the sense of Einstein's famous quote) ...


Most of the comments in the posted source are either unnecessary
or misplaced. More specifically, comments related to _interface_
should be separated from _implementation_ (and all comments
pertaining to interface should come before any code containing
implementation). A developer wanting to use this package should
be able to read just that part describing the interface, and
nothing in the implementation, and know enough to start using it.

 

[Shao Miller]

Where one uses macros instead of jump statements (though the actual
statements _could_ be overridden, theoretically) and where one uses
the 'HAS_CLEANUP' macro within whatever scopes require resource
cleanup.

Feedback welcome and appreciated, as always.
[snip posted source]

It would be TREMENDOUSLY helpful to take a different approach to
documentation style. For example,

/*
Several prose paragraphs, giving problem statement,
basic approach, perhaps a short list of primary
functions or other elements.
*/

Ok. Thanks.

#if 0 /* Example uses */

... some simple but illustrative examples ...

#endif

Ah yes. Good idea.

/* Description of interface elements, entry points:

name1 discussion/explanation

name2 discussion/explanation

... other macro names, type names, function names, etc ...

*/

Is this really different than what I've got? I guess you are suggesting
that it all be grouped together within a single comment?

... declarations (without contents) of public struct types ...

Well, yes. I normally do:

typedef struct s_xxx_ s_xxx

... public function prototypes ...

And then use 's_xxx' in the public function types and/or declarations.
Since this example had a single, simple, public structure type, the
definition was nearby (just above it, if I recall correctly).

... now give code with "as few comments as necessary" ...

Is that really different than what I've got?

... (in the sense of Einstein's famous quote) ...

Something about "dice"? Just kidding.

Most of the comments in the posted source are either unnecessary
or misplaced.

Uh oh. "Most"?

More specifically, comments related to _interface_
should be separated from _implementation_ (and all comments
pertaining to interface should come before any code containing
implementation).

Hmmm...

'cleanup.h' defines some macros. Yes, the comments are near the
implementation. So ok, I'll try to remember to take your advice for the
next time, and throw macro documentation somewhere separately.

If you look at any of my other posted code examples, you'll find that
comments precede function declarations (or types) in the header. So I
do [try to] keep that as a habit. If I understand your criticism here,
it's about not using that habit for these macros (the functions are all
private).

A developer wanting to use this package should
be able to read just that part describing the interface, and
nothing in the implementation, and know enough to start using it.

I really thought a developer might benefit from seeing the brief macro
implementations right there, but perhaps that's better off as requiring
some "scrolling to the details."

Thanks for reviewing! Perhaps the improvements you suggest could make
the code more accessible and facilitate review of the functionality.

 

[Tim Rentsch]

Where one uses macros instead of jump statements (though the actual
statements _could_ be overridden, theoretically) and where one uses
the 'HAS_CLEANUP' macro within whatever scopes require resource
cleanup.

Feedback welcome and appreciated, as always.
[snip posted source]

It would be TREMENDOUSLY helpful to take a different approach to
documentation style. For example,

/*
Several prose paragraphs, giving problem statement,
basic approach, perhaps a short list of primary
functions or other elements.
*/

Ok. Thanks.

#if 0 /* Example uses */

... some simple but illustrative examples ...

#endif

Ah yes. Good idea.

/* Description of interface elements, entry points:

name1 discussion/explanation

name2 discussion/explanation

... other macro names, type names, function names, etc ...

*/

Is this really different than what I've got? I guess you are
suggesting that it all be grouped together within a single comment?

Yes, and laid out in this way, and giving just a
description of interface, with no implementation.

Well, yes. I normally do:

typedef struct s_xxx_ s_xxx

The key thing is that the types be declared, not whether
typedef's are used to declare them. (If there are other
relevant types that's aren't struct/unions, typedefs for
those should be here also.)

And then use 's_xxx' in the public function types and/or
declarations. Since this example had a single, simple, public
structure type, the definition was nearby (just above it, if I recall
correctly).

The function prototypes, which are interface, should come
before the definition of structure contents, which are
implementation (as I understand what you are trying to
provide).

Is that really different than what I've got?

Yes, emphatically yes.


[snip]

Uh oh. "Most"?

Yes, it's an English word meaning 'the majority of'.

More specifically, comments related to _interface_
should be separated from _implementation_ (and all comments
pertaining to interface should come before any code containing
implementation).

Hmmm...

'cleanup.h' defines some macros. Yes, the comments are near the
implementation. So ok, I'll try to remember to take your advice for
the next time, and throw macro documentation somewhere separately.

If you look at any of my other posted code examples, you'll find that
comments precede function declarations (or types) in the header. So I
do [try to] keep that as a habit. If I understand your criticism
here, it's about not using that habit for these macros (the functions
are all private).

My suggestions are primarily generically these:

1. All description of interface should precede all source code
supplying implementation. Naturally some of the implementation
source code will duplicate some information already given in
the interface section (such as tags of struct types), but that
consequence is secondary to the principle of giving interface
separately from implementation.

2. Layout and presentation are important; the examples I
gave offer some (what I think are) improvements.

3. Even within the implementation section, a good prose
summary description generally provides more value than
lots of individual, highly localized comments. "A good
overview is worth more than 10,000 per-line comments."

A developer wanting to use this package should
be able to read just that part describing the interface, and
nothing in the implementation, and know enough to start using it.

I really thought a developer might benefit from seeing the brief macro
implementations right there, [snip]

Just the opposite: a developer will benefit from _not_ seeing
the implementation as much as that is possible. Look up
Parnas's papers on information hiding, and read Fred Brooks's
comments related to that in the 20th anniversary edition of "The
Mythical Man-Month".

Thanks for reviewing! [snip]

You're welcome, I'm glad you found it helpful.