From owner-svn-src-head@FreeBSD.ORG Sun Jun 19 11:55:22 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 F05881065672; Sun, 19 Jun 2011 11:55:22 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-pv0-f182.google.com (mail-pv0-f182.google.com [74.125.83.182]) by mx1.freebsd.org (Postfix) with ESMTP id A52DD8FC14; Sun, 19 Jun 2011 11:55:22 +0000 (UTC) Received: by pvg11 with SMTP id 11so1061030pvg.13 for ; Sun, 19 Jun 2011 04:55:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:references:in-reply-to:mime-version :content-transfer-encoding:content-type:message-id:cc:x-mailer:from :subject:date:to; bh=YruydtxjfqY19tiXunjg9rJNl+80F2F/NqJZ6TlJ4+M=; b=dy5hLi82yxhrziadhydF6adv3XJ5waZQPAi5GcEFgIQU0C8S3wqax43Q9jmeSSHVOs oiD2F8dFizaWx3csdLo9p0raUKI2TCZoY79uSLGwCAM0XQpeFik7bOMq7oAJ5PDDDCIh cVBRY29mJBxRsqj+jqqKW7QXurPqiJaCIvcR0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=references:in-reply-to:mime-version:content-transfer-encoding :content-type:message-id:cc:x-mailer:from:subject:date:to; b=rLe6nXLgSTHeZ3ZxH+eSTY/wQEDD36bavv8LDKwZBPzft3RPHxGGop6Y0k14aCytAs KHsZIZTebcLuqqdl+LYjMtu/xiCXXSx6RZF6MM/nb7ImOAseDtZxRtnrdN/QFoqv/I2c 5JCpiqEryjGrqvDbW86y2ofn6JcyDvpUxFAm0= Received: by 10.68.34.72 with SMTP id x8mr1624697pbi.312.1308484522168; Sun, 19 Jun 2011 04:55:22 -0700 (PDT) Received: from [192.168.20.56] (c-24-6-49-154.hsd1.ca.comcast.net [24.6.49.154]) by mx.google.com with ESMTPS id m9sm2588342pbd.39.2011.06.19.04.55.20 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 19 Jun 2011 04:55:21 -0700 (PDT) References: <201106181356.p5IDuXhW044171@svn.freebsd.org> <20110619.114704.59640143160110864.nyan@FreeBSD.org> <20110619193320.Y917@besplex.bde.org> In-Reply-To: <20110619193320.Y917@besplex.bde.org> Mime-Version: 1.0 (iPhone Mail 8J2) Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii Message-Id: <2FF8921B-04E8-43CA-9140-1DEB0933C8E3@gmail.com> X-Mailer: iPhone Mail (8J2) From: Garrett Cooper Date: Sun, 19 Jun 2011 04:55:16 -0700 To: Bruce Evans Cc: "benl@FreeBSD.org" , "svn-src-head@FreeBSD.org" , "svn-src-all@FreeBSD.org" , "src-committers@FreeBSD.org" , TAKAHASHI Yoshihiro Subject: Re: svn commit: r223262 - in head: cddl/contrib/opensolaris/lib/libdtrace/common contrib/binutils/bfd contrib/binutils/gas contrib/binutils/gas/config contrib/binutils/ld contrib/binutils/opcodes contr... 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: Sun, 19 Jun 2011 11:55:23 -0000 On Jun 19, 2011, at 4:26 AM, Bruce Evans wrote: > On Sun, 19 Jun 2011, TAKAHASHI Yoshihiro wrote: >=20 >> In article <201106181356.p5IDuXhW044171@svn.freebsd.org> >> Ben Laurie writes: >>>=20 >>> Log: >>> Fix clang warnings. >>>=20 >>> Modified: head/sys/sys/diskpc98.h >>> =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/sys/diskpc98.h Sat Jun 18 13:54:36 2011 (r223261) >>> +++ head/sys/sys/diskpc98.h Sat Jun 18 13:56:33 2011 (r223262) >>> @@ -36,8 +36,11 @@ >>> #include >>>=20 >>> #define DOSBBSECTOR 0 /* DOS boot block relative sector number *= / >>> +#undef DOSPARTOFF >>> #define DOSPARTOFF 0 >>> +#undef DOSPARTSIZE >>> #define DOSPARTSIZE 32 >>> +#undef NDOSPART >>> #define NDOSPART 16 >>> #define DOSMAGICOFFSET 510 >>> #define DOSMAGIC 0xAA55 >>> @@ -52,6 +55,7 @@ >>>=20 >>> #define DOSMID_386BSD (PC98_MID_386BSD | PC98_MID_BOOTABLE) >>> #define DOSSID_386BSD (PC98_SID_386BSD | PC98_SID_ACTIVE) >>> +#undef DOSPTYP_386BSD >>> #define DOSPTYP_386BSD (DOSSID_386BSD << 8 | DOSMID_386BSD) >>>=20 >>> struct pc98_partition { >>>=20 >>=20 >> I wonder why this is needed, and why only for diskpc98.h, not >> diskmbr.h. >=20 > This is needed to break the warning even more. There are are enormous > number of layers of brokenness: >=20 > o These header files unfortunately define some ioctls (just 1), so > kdump/mkioctls generates an include of both in ioctl.c. This causes > conflicts. The conflicts should cause errors (though the conflicting > definitions aren't actually used in ioctl.c), but they only generates > warnings. >=20 > o If you compile ioctl.c standalone to test this, then it compiles with > no warnings with both gcc and clang, since -Wno-system-headers is the > default for gcc and apparently for clang too. However, for world builds > there is magic that results in being > $(realpath /usr/obj)/src/tmp/usr/include/sys instead of just > /usr/include/sys. I'm not sure if clang only is confused by this so > that -Wno-system-headers doesn't break the warnings for clang only, > or if the warnings have always been happening and they are just more > obvious with clang colorizing them. >=20 > o The build system knows about -Wno-system-headers breaking warnings, so > it puts -Wsystem-headers in CFLAGS to turn this off. But it only does > this at WARNS >=3D 1, and kdump still uses WARNS =3D 0. >=20 > o Poor structuring of the disk headers. I've always thought that > diskmbr.h shouldn't exist. It doesn't describe "the" disk mbr > structure, but only the "i386" one. It should have been named > something like diski386.h. The current problem shows that these > files should have been purely MD so that they can be kept separate. > Their names should have been something like . > Or for simplicity, keep all of their definitions with ifdefs in > like they used to be as recently as FreeBSD-4. > disklabel.h still has some ugly unsorted MD ifdefs (just 2, for > LABELSECTOR and LABELOFFSET). Of course, definitions for MBRs > should never have been in disklabel.h. For simplicity with fewer > hacks, put all the MBR definitions with ifdefs in . > The current problem shows that many ifdefs are needed with the > current structuring anyway. We only escape having to ifdef everything > in both of the files (or complications in mkioctls) because some names > are different (e.g., struct pc98_partition instead of struct > dos_partition, and more importantly, DIOCSPC98 instead of DIOCSMBR -- > the latter allows both ioctls to be defined in ioctl.c, though only > the pc98 is normally used on pc98. BTW, can pc98 handle i386-formatted > disks? MacOS supports i386-mbr formatted USB drives better than > WinXP does -- WinXP can only mount FAT32 partitions in MBR slot 1, > though it can recognize them in any slot. Such support is easiest > if all the mbr ioctls are available in all arches. But I now think > that depending on any disk ioctl in an application like fdisk or > newfs is a bug. Such applications should be able to work on any > "direct addressable" file (including regular files and foreign > disks). Some Linux disk applications now work better on FreeBSD than > FreeBSD ones do, since they don't depend so much on ioctls :-(). >=20 > This is only "needed" for diskpc98.h because 'm' is less than 'p', so > diskmbr.h is always included before diskpc98.h. I thought at first > that the order of the includes could not be depended on, because the > find(1) in mkioctls produced output in (unsorted) tree traversal order. > But the find is actually on "*" in each directory in the include path, > so the shell will sort the includes alphabetically within each directory. > I now remember that this is intentional, to get the same breakage as > an application which complies with style(9) will get if there are any > bogus ordering requirements for directories that are not accidentally > satisified by keeping the includes sorted. See the /* XXX obnoxious > prerequisites. */ section in mkioctls. This intentionally includes > a few headers out of order, to work around cases where the normal > order doesn't work. One of these headers is disklabel.h, which > might not be needed there any more now that it has been cleaned up. If my ktrace WARNS 0 -> 6 patch had been reviewed that I submitted some week= s ago, then this would be moot. It's not a perfect solution, but it worked. Thanks, -Garrett=