[PATCH] Add compatibility <sys/io.h>

Bruce Evans brde at optusnet.com.au
Mon Mar 19 10:53:23 UTC 2012


On Mon, 19 Mar 2012, Robert Millan wrote:

> El 12 de març de 2012 3:18, Bruce Evans <brde at optusnet.com.au> ha escrit:
>> I would prefer to make it fail to build if it gets the arg order wrong,
>> but I don't see how to do that, since both args are integers.
>
> I don't think you can. E.g. consider:
>
> outb (0x60, 0x61);
>
> then only semantical analysis could tell.
>
>>> Looks good.  So you suggest we tell userspace users to switch to
>>> bus_space_write_*?
>>
>>
>> The problem with that is that if you don't do the switch yourself then
>> most users won't even know that it is necessary.  You have to remove
>> the functionaility in cpufunc.h and/or add messy userland ifdefs as
>> well as messy kernel ifdefs to unremove it, so that users who don't
>> know what they are doing and which you haven't adjusted get warned
>> by build failures.  All users that knew what they were doing have to
>> do it differently.
>
> Okay, I see your point. Maybe we can try a conservative approach and
> just issue warnings. How does this look for a start?

% Index: sys/i386/include/cpufunc.h
% ===================================================================
% --- sys/i386/include/cpufunc.h	(revision 233095)
% +++ sys/i386/include/cpufunc.h	(working copy)
% @@ -42,6 +42,10 @@
%  #error this file needs sys/cdefs.h as a prerequisite
%  #endif
% 
% +#ifndef _KERNEL
% +#warning "No user-serviceable parts inside. For user-space I/O, use the bus_space(9) family of functions."
% +#endif
% +

Too many style bugs for me :-) :
- line too long
- multiple sentences.  These are unusual in error messages, and help give
   the previous bug
- terminated message.  Error messages are not terminated with a ".",
   except possibly if they have multiple sentences, each terminated with
   a ".".  I don't know the exact rule for more normal use.  This sentence
   is terminated, unlike the last one for the previous point.
- Capitalized sentences.  Again it is unclear what should be capitalized
   for multiple sentences.  The one that started this point is capitalized,
   unlike all the others here except for the one at the end of the list.
- sentence break of 1 space instead of 2.

All of these style bugs are avoided in all user-anti-serviceable messages
in sys/*.h.  All 8 there say '#error "no user-serviceable parts inside"',
except 1 says "no assembler-serviceable parts inside", and 5 misspell
"serviceable" as "servicable".  This phrasing is of course a joke.  I
know prefer the more correct "no user-serving parts inside".

#warning must be used instead of #error here since we don't want to break
this use completely, but there is a technical problem with this.
#warning is only a gcc extension.  This is handled in gratuitously
different horrible ways in all places that #warning is used in sys/*.h:

% dir.h:
% #ifdef __CC_SUPPORTS_WARNING
% #warning "The information in this file should be obtained from <dirent.h>"
% #warning "and is provided solely (and temporarily) for backward compatibility."
% #endif

The condition for #warning being available is given by the rather
over-engineered feature test macro __CC_SUPPORTS_WARNING.  To make the
above correct, this file grew an otherwise-unnecessary include of
<sys/cdefs.h> just before the above.

The warning message is rather formal and verbose.  It is formatted nicely
enough using multiple #warning statements to avoid the long lines.  gcc
prints the multiple #warning statements even with -Werror.  It even prints
multiple #error statements and doesn't abort on the first one.

% syslimits.h:
% #if !defined(_KERNEL) && !defined(_LIMITS_H_) && !defined(_SYS_PARAM_H_)
% #ifndef _SYS_CDEFS_H_
% #error this file needs sys/cdefs.h as a prerequisite
% #endif
% #ifdef __CC_SUPPORTS_WARNING
% #warning "No user-serviceable parts inside."
% #endif
% #endif

Now the ifdefs are even more horrible.  The #error for not including
sys/cdefs.h is bogus.  We are trying to trap the error of including
this file directly.  It must only be included by <limits.h> or
<sys/param.h>.  This has nothing to do with sys/cdefs.h, but we print
a #error about the latter.  The primary warning about the former is
the usual cryptic "!user-serviceable" one, except it now has style
bugs.  This ifdef tangle is apparently the result of:
- using #warning instead of #error for no reason
- adding the __CC_SUPPORTS_WARNING ifdef to hide this bug.  This ifdef
   was originally spelled __GNUC__
- when converting from the __GNUC__ spelling to __CC_SUPPORTS_WARNING,
   sys/cdefs.h became a prerequisite, but instead of just including it,
   an ifdef for its presence was added and the #error message for that
- the _KERNEL ifdef is also bogus.

Untangling this and fixing the message gives:

% #if !defined(_LIMITS_H_) && !defined(_SYS_PARAM_H_)
% #error "no user-serviceable parts inside"
% #endif

or the message could be decrytped to

#error "this file must only be included directly by <limits.h> or <sys/param.h>"

but I don't want to spell out the implementation details like this in all
#error messages.

% timeb.h:
% #ifdef __GNUC__
% #warning "this file includes <sys/timeb.h> which is deprecated"
% #endif

The old spelling of __CC_SUPPORTS_WARNING here is an anachronism.
The other ifdefs on #warning date from 1997 and 2002, but were changed
to the new spelling in 2005.  This one dates from 2010 but hasn't
caught up with the 2005 change.

I fixed the spelling errors long ago in my version, so I didn't notice
them at first when I grepped for them while writing this.  After adding
a few more of these messages, I now have the following in sys/*.h:

%%%
_task.h:#error "no user-servicable parts inside"
eventhandler.h:#error "no user-serviceable parts inside"
libkern.h:#error "no user-serviceable parts inside"
mutex.h:#error "no assembler-serviceable parts inside"
pcpu.h:#error "no user-serviceable parts inside"
pcpu.h:#error "no assembler-serviceable parts inside"
smp.h:#error "no user-serviceable parts inside"
smp.h:#error "no assembler-serviceable parts inside"
syslimits.h:#error "no user-serviceable parts inside"
taskqueue.h:#error "no user-serviceable parts inside"
timetc.h:#error "no user-serviceable parts inside"
%%%

Kernel headers really shouldn't need these, but adding just one
takes a lot of work since it tends to expose kernel headers
leaking to userland with full kernel pollution like kernel
mutexes and locks.

The "no assembler-serviceable parts inside" are even less needed.
I added them mainly in places where there were bogus LOCORE ifdefs,
to detect any cases where asm files actually still include C headers
(C except for a tiny part ifdefed by LOCORE).  genassym mostly handles
this better although it takes a few more seconds to hack on.

Bruce


More information about the freebsd-arch mailing list