From owner-freebsd-arch@FreeBSD.ORG Wed Apr 2 09:10:47 2008 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id ABD0C106566B for ; Wed, 2 Apr 2008 09:10:47 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from hosted.kievnet.com (hosted.kievnet.com [193.138.144.10]) by mx1.freebsd.org (Postfix) with ESMTP id 694368FC1A for ; Wed, 2 Apr 2008 09:10:47 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from localhost ([127.0.0.1] helo=edge.pp.kiev.ua) by hosted.kievnet.com with esmtpa (Exim 4.62) (envelope-from ) id 1JgybC-0004fk-41 for freebsd-arch@freebsd.org; Wed, 02 Apr 2008 11:45:38 +0300 Message-ID: <47F347B1.2020509@icyb.net.ua> Date: Wed, 02 Apr 2008 11:45:37 +0300 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.12 (X11/20080320) MIME-Version: 1.0 To: freebsd-arch@freebsd.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: kobj method signature/prototype checking/enforcement X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Apr 2008 09:10:47 -0000 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