kobj method signature/prototype checking/enforcement

Andriy Gapon avg at icyb.net.ua
Wed Apr 2 02:10:47 PDT 2008


As you are most probably aware, currently there is no
checking/enforcement for signatures of functions implementing kobj
methods. Internally all function pointers are stored as pointers to 'int
f(void)', and they are cast to and from as needed.
So, for example, if you set a function 'char * g(void **)' as
device_probe method then the compiler will compile everything just fine,
it will be only at run-time that you will get a trouble because of
mismatching arguments.

I propose to defend against this problem using the following macro for
KOBJMETHOD:
#define KOBJMETHOD(NAME, FUNC) \
{ &NAME##_desc, (kobjop_t) (FUNC != (NAME##_t *)NULL ? FUNC : NULL) }

This is an idea behind it:
1. the comparison expression is a NOP, its result is always the same as
(kobjop_t)FUNC
2. the expression is evaluated at compile time, so it doesn't create any
run-time overhead or binary differences
3. purpose of expression is to make use of GCC feature to warn about
comparing "distinct pointer types"

I tested this change with 6.3-RELEASE sources. It revealed a number of
signature mismatches in different places. Obviously all of them are
quite harmless - otherwise they would be already discovered in a hard
way (by people bitten).

Here's a general overview of issues discovered:
1. integer parameters differing in signedness (totally harmless, I think)
2. using void return type instead of int, usually for device_shutdown
method (not sure about this one)
3. using int return type instead of specific size integer return type,
typically for sound channel interface methods
4. 'char *' parameter instead of 'const char *' parameter (potentially
can result in future problems)
5. significantly different signatures for several "dummy" methods that
do not actually use any of the parameters and simply print a message or
panic.

While the above issues are quite harmless, I still think that adding
such a checking code is a good thing. It will help with new code
development and it will help general code quality and maintenance.

Unfortunately I don't have my FreeBSD development environment quite set
up (yet) for large scale development, so at this point I can not provide
a patch for HEAD that would fix all the build breakages (on all the
platforms) that would be caused by the proposed change (when -Werror is
in effect).

-- 
Andriy Gapon


More information about the freebsd-arch mailing list