Date: Sun, 19 Jun 2011 04:55:16 -0700 From: Garrett Cooper <yanegomi@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: "benl@FreeBSD.org" <benl@FreeBSD.org>, "svn-src-head@FreeBSD.org" <svn-src-head@FreeBSD.org>, "svn-src-all@FreeBSD.org" <svn-src-all@FreeBSD.org>, "src-committers@FreeBSD.org" <src-committers@FreeBSD.org>, TAKAHASHI Yoshihiro <nyan@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... Message-ID: <2FF8921B-04E8-43CA-9140-1DEB0933C8E3@gmail.com> In-Reply-To: <20110619193320.Y917@besplex.bde.org> References: <201106181356.p5IDuXhW044171@svn.freebsd.org> <20110619.114704.59640143160110864.nyan@FreeBSD.org> <20110619193320.Y917@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jun 19, 2011, at 4:26 AM, Bruce Evans <brde@optusnet.com.au> wrote:
> On Sun, 19 Jun 2011, TAKAHASHI Yoshihiro wrote:
>=20
>> In article <201106181356.p5IDuXhW044171@svn.freebsd.org>
>> Ben Laurie <benl@freebsd.org> 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 <sys/ioccom.h>
>>>=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 <sys> 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 <machine/diskmbr.h>.
> Or for simplicity, keep all of their definitions with ifdefs in
> <sys/disklabel.h> 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 <sys/diskmbr.h>.
> 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=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2FF8921B-04E8-43CA-9140-1DEB0933C8E3>