tdb API issues

[simo]

On Sat, 2009-04-04 at 11:16 -0700, Howard Chu wrote:
> Also just a comment on coding practice: it's better to typedef actual objects, 
> not pointers to objects. E.g. in tdb.h:
> 
> typedef int (*tdb_traverse_func)(struct tdb_context *, TDB_DATA, TDB_DATA, 
> void *);
> typedef void (*tdb_log_func)(struct tdb_context *, enum tdb_debug_level, const 
> char *, ...) PRINTF_ATTRIBUTE(3, 4);
> typedef unsigned int (*tdb_hash_func)(TDB_DATA *key);
> 
> struct tdb_logging_context {
>          tdb_log_func log_fn;
>          void *log_private;
> };
> 
> 
> It would have been better to use
> 
> typedef int (tdb_traverse_func)(struct tdb_context *, TDB_DATA, TDB_DATA, void *);
> typedef void (tdb_log_func)(struct tdb_context *, enum tdb_debug_level, const 
> char *, ...) PRINTF_ATTRIBUTE(3, 4);
> 
> struct tdb_logging_context {
>          tdb_log_func *log_fn;
>          void *log_private;
> };
> 
> This way you can use the actual typedef in function declarations, and the 
> compiler will generate an error if you get the function signature wrong at the 
> point the function is defined:
> 
> extern tdb_traverse_func my_traverser;
> 
> ...
> int my_traverser(struct tdb_context *foo, TDB_DATA key) /* oops */
> {
>   ...
> }
> 
> With the pointer typedef, you'll only get a warning at the point of use, so 
> it'll successfully build even if your code is broken.

I guess it's too late to change existing functions, but we should
definitely keep it in mind for future declarations.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>