From owner-svn-src-all@FreeBSD.ORG Sun Jun 19 11:26:46 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C47121065674; Sun, 19 Jun 2011 11:26:46 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail02.syd.optusnet.com.au (mail02.syd.optusnet.com.au [211.29.132.183]) by mx1.freebsd.org (Postfix) with ESMTP id 3BCA08FC1D; Sun, 19 Jun 2011 11:26:45 +0000 (UTC) Received: from c122-106-165-191.carlnfd1.nsw.optusnet.com.au (c122-106-165-191.carlnfd1.nsw.optusnet.com.au [122.106.165.191]) by mail02.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id p5JBQf11007476 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 19 Jun 2011 21:26:42 +1000 Date: Sun, 19 Jun 2011 21:26:41 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: TAKAHASHI Yoshihiro In-Reply-To: <20110619.114704.59640143160110864.nyan@FreeBSD.org> Message-ID: <20110619193320.Y917@besplex.bde.org> References: <201106181356.p5IDuXhW044171@svn.freebsd.org> <20110619.114704.59640143160110864.nyan@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: benl@FreeBSD.org, svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org 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-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Jun 2011 11:26:46 -0000 On Sun, 19 Jun 2011, TAKAHASHI Yoshihiro wrote: > In article <201106181356.p5IDuXhW044171@svn.freebsd.org> > Ben Laurie writes: >> >> Log: >> Fix clang warnings. >> >> Modified: head/sys/sys/diskpc98.h >> ============================================================================== >> --- 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 >> >> #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 @@ >> >> #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) >> >> struct pc98_partition { >> > > I wonder why this is needed, and why only for diskpc98.h, not > diskmbr.h. This is needed to break the warning even more. There are are enormous number of layers of brokenness: 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. 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. 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 >= 1, and kdump still uses WARNS = 0. 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 :-(). 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. Bruce