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...
Garrett Cooper
yanegomi at gmail.com
Sun Jun 19 11:55:23 UTC 2011
On Jun 19, 2011, at 4:26 AM, Bruce Evans <brde at optusnet.com.au> wrote:
> On Sun, 19 Jun 2011, TAKAHASHI Yoshihiro wrote:
>
>> In article <201106181356.p5IDuXhW044171 at svn.freebsd.org>
>> Ben Laurie <benl at freebsd.org> 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 <sys/ioccom.h>
>>>
>>> #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 <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.
>
> 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 <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 :-().
>
> 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 weeks ago, then this would be moot. It's not a perfect solution, but it worked.
Thanks,
-Garrett
More information about the svn-src-all
mailing list