From owner-svn-src-head@FreeBSD.ORG Fri Jan 14 15:23:19 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E32E4106564A; Fri, 14 Jan 2011 15:23:19 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id 861648FC16; Fri, 14 Jan 2011 15:23:19 +0000 (UTC) Received: by iwn39 with SMTP id 39so2608277iwn.13 for ; Fri, 14 Jan 2011 07:23:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=nmYL/QfQ5gz+P3YI6I0LqRSeuT5jxyhsy23jqvhc2CE=; b=i4c56VcLgEQwUvYlLwxG9r6lSj3PvHzEoRKH6nURklFc2WzpzZA71w64wZHfdqgCiC cm7WuO2cLEgl6+ojn50fJxE/WnjyvWTxLJy65nB8qP/a7Cl3MKHGrcwysEovMZGrgPqO jRtwWb9JKVAgB+lzLI4SSkxd7cyItmQyIzJ84= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=uE6sq7YWn1ZhzHovY0DAe/Qd6nHkzJiY4BRj1fzmV4ASFWy39WXsFaEQn/bO2D/azw VZUAYa2/yxmzQNroo5slHFeEKVCsoJu4ZakFUkT4dhv7onAFA3V3X0opwxQBpcvwuCUJ IF49NCvKRYvy786DKlC5ujUfz49dG3ngcdl4o= MIME-Version: 1.0 Received: by 10.231.40.3 with SMTP id i3mr789148ibe.129.1295018576373; Fri, 14 Jan 2011 07:22:56 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.231.160.147 with HTTP; Fri, 14 Jan 2011 07:22:56 -0800 (PST) In-Reply-To: <20110114162142.M27877@besplex.bde.org> References: <201101131820.p0DIKXip059402@svn.freebsd.org> <20110114162142.M27877@besplex.bde.org> Date: Fri, 14 Jan 2011 07:22:56 -0800 X-Google-Sender-Auth: jR1kAjVfbZoOMrVDiv9qSRmPmcw Message-ID: From: mdf@FreeBSD.org To: Bruce Evans Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r217369 - in head/sys: cam/scsi sys X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Jan 2011 15:23:20 -0000 On Thu, Jan 13, 2011 at 9:56 PM, Bruce Evans wrote: > On Thu, 13 Jan 2011, Matthew D Fleming wrote: > >> Log: >> =A0Add a 64-bit hex-printed sysctl(9) since there is at least one place = in >> =A0the code that wanted it. =A0It is named X64 rather than XQUAD since t= he >> =A0quad name is a historical abomination that should not be perpetuated. > > :-). =A0It is only long long that is abominable. =A0Both are historical. > > I think these X formats shouldn't exist, even as defaults. =A0Instead, > sysctl(8) should have option(s) to control the format. =A0I'm used to > typing lots of "p/x"s to get hex formatting in gdb. =A0There is little > reason for sysctl(8) to have finer-grained control than gdb (sysctl > now has hard-coded defaults instead of control). > > Now with stricter type checking, even formats for integers are redundant. > The CTLTYPE now always matches the type, and the format should always > match the type. =A0The space wasted for the format is 1 pointer plus the > string. There are several issues here. First, for OPAQUE types the string is interpreted by sysctl(8) to know how to interpret the binary blob. Second, anyone can still use SYSCTL_OID(... CTLTYPE_INT, ..., "QU") and there's not a lot that can be done to check it. Locally I have another patch for the various sysctl_handle_foo that asserts the CTLTYPE_FOO matches the handler, but looking around the FreeBSD code there's plenty of hand-rolled instances that won't pass; some of these come from a SYSCTL_PROC setup. Perhaps we could start with a warning message and upgrade to a KASSERT later. Thirdly, at the moment sysctl(8) doesn't fetch the access flags and type specifier and only looks at the string. This is also fixable. I do agree that the incredibly limited use of XINT, XLONG, and the one use of X64 mean that it's not a widely used feature, and can probably be better done with a -x flag to sysctl(8). Most bitflags are still added as SYSCTL_UINT, so one has to re-interpret the value anyways to see which bits are set. > Perhaps there are some remaining type errors involving the kernel type > being signed when it should be unsigned, or vice versa. =A0This could > be "fixed" by mis-specifying the format with a U, or vice versa. =A0Since > the specification is usually done by invoking a SYSCTL*() macro, most > such fixes, if any, would have been undone by changing the macro to > match the type, and it would take fixing the kernel type to fix the > userland format. The framework is in place to fix the obvious type errors for scalar types, but I haven't yet finished making the kernel compile cleanly in universe mode. Until then the check is still if 0'd out. Thanks, matthew > >> Modified: head/sys/cam/scsi/scsi_da.c >> >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- head/sys/cam/scsi/scsi_da.c Thu Jan 13 18:20:27 2011 =A0 =A0 =A0 =A0= (r217368) >> +++ head/sys/cam/scsi/scsi_da.c Thu Jan 13 18:20:33 2011 =A0 =A0 =A0 =A0= (r217369) >> @@ -1127,9 +1127,9 @@ dasysctlinit(void *context, int pending) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct ccb_trans_settings_fc *fc =3D &cts= .xport_specific.fc; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (fc->valid & CTS_FC_VALID_WWPN) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0softc->wwpn =3D fc->wwpn; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 SYSCTL_ADD_XLONG(&softc->s= ysctl_ctx, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 SYSCTL_ADD_X64(&softc->sys= ctl_ctx, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0SYSCTL_CHILDREN(s= oftc->sysctl_tree), >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 OID_AUTO, "wwpn", = CTLTYPE_QUAD | CTLFLAG_RD, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 OID_AUTO, "wwpn", = CTLFLAG_RD, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&softc->wwpn, "Wo= rld Wide Port Name"); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} >> =A0 =A0 =A0 =A0} >> > > Hmm, forcing hex might be best for flags (but I'll ask for binary then :-= ) > and for mac addresses, but not for inet4 addresses. =A0I don't know what = sort > of address this is. > > Bruce >