svn commit: r232261 - in head/sys: amd64/include i386/include pc98/include x86/include

[Bruce Evans]

On Thu, 1 Mar 2012, Tijl Coosemans wrote:

> On Wednesday 29 February 2012 00:26:43 Bruce Evans wrote:

> The __clock_t case is a perfect example of how duplication leads to
> differentiation and differentiation to portability issues. Currently
> clock(3) only starts to return negative values after about six months
> of cpu time, so I think it's relative safe to change the type to
> uint32_t. I haven't checked the other cases where this type is used
> though.

This reminds me that clock_t is still essentially useless.  The system
has an accuracy of sub-microseconds for many times, but clock(3) is
dumbed down to give give the semi-historical accuracy of 1/128 (1/60
miht be fully historical).  The only other use of clock_t in FreeBSD
is the SYSV compatibility cruft API times(3).  This is a larger dumbing
down of getrusage(3).  Without the dumbing down, CLOCKS_PER_SEC would
be something like 1e9 and a 32-bit clock_t would overflow after 4
seconds, so it would have to be 64 bits.

Grepping for clock_t shows some nice bugs in pthread_condattr(3).  It
declares some parameters as "clock_t * restrict clock_id" and
"clock_t clock_id".  This is nonsense, since clock ids aren't clock
ticks.  POSIX specifies correct declarations "clockid_t * restrict
clock_id" and "clockid_t clock_id".  <sys/pthread.h> matches POSIX
except it doesn't say "restrict".  The man page is missing
pthread_getcpuclockid() but has the other 2 (with type errors).

>>> Copied and modified: head/sys/x86/include/_types.h (from r232259, head/sys/amd64/include/_types.h)
>>> ==============================================================================
>>> --- head/sys/amd64/include/_types.h	Tue Feb 28 15:52:01 2012	(r232259, copy source)
>>> +++ head/sys/x86/include/_types.h	Tue Feb 28 18:15:28 2012	(r232261)
>>> @@ -54,19 +54,41 @@ typedef	short			__int16_t;
>>> +#else
>>> +#ifndef lint
>>> +__extension__
>>
>> An old bug -- work around broken lints.  Although messy, this is not
>> messy enough to be correct -- __extension__ is a hard-coded gccism.
>> Elsewhere, in much less important code than this, there are messy
>> ifdefs to avoid hard-coded gccisms.
>
> Maybe define __extension for lint and various compilers in cdefs.h?

Wouldn't lint still trip over the actual extension?  lint needs to
understand __extension__ more than most gcc things so that we can
kill its warnings for things that it is further from understanding.
Hopefully this can be done less obtrusively than with ifdefs.

>>> @@ -82,8 +105,18 @@ typedef	__uint64_t	__size_t;		/* sizeof(
>>> typedef	__int64_t	__ssize_t;		/* byte count or error */
>>> typedef	__int64_t	__time_t;		/* time()... */
>>> typedef	__uint64_t	__uintfptr_t;
>>> -typedef	__uint64_t	__uintmax_t;
>>> typedef	__uint64_t	__uintptr_t;
>>> +#else
>>> +typedef	__int32_t	__ptrdiff_t;
>>> +typedef	__int32_t	__register_t;
>>> +typedef	__int32_t	__segsz_t;
>>> +typedef	__uint32_t	__size_t;
>>> +typedef	__int32_t	__ssize_t;
>>> +typedef	__int32_t	__time_t;
>>> +typedef	__uint32_t	__uintfptr_t;
>>> +typedef	__uint32_t	__uintptr_t;
>>> +#endif
>>
>> [unsigned] long would work without ifdefs for all of these, since all
>> these expanded naturally to the register width.  Perhaps a better way,
>> which also works for i386's with correctly-sized longs, is to define
>> almost everything in terms of registers -- as __[u]register_t.
>
> register_t is a machine dependent type and the others are LP64/ILP32
> dependent, so they're not strictly related. MIPS N32 is an example of
> that.

It will work for most arches.  It's only if you want to put everything in
1 "MI" types.h file that the details are hard to know.

> As for using long, I'm not sure. It has the same size as int, but they
> aren't the same in format strings.

Yes, since these types escape to MI APIs, the longs give slightly larger
problems for printf formats (you shouldn't print any foo_t using %d,
but it mostly works for 32-bit types, and changing the basic type to
long would break this).

>>> +typedef	__uint32_t	__vm_paddr_t;
>>> +#endif
>>> +typedef	__uint32_t	__vm_size_t;
>>> +#endif
>>> +typedef	__int64_t	__vm_ooffset_t;
>>> +typedef	__uint64_t	__vm_pindex_t;
>>
>> Similarly.  The patch, and possibly the ifdefs, are hard to read here.
>> There's a nested ifdef for PAE.  PAE doesn't apply for amd64.  The
>> above assumes that the cases where it doesn't apply are classified
>> by !_LP64.
>
> I'm not sure yet about what should happen with these vm types. I'm not
> entirely convinced yet that all these "invisible" types are really
> necessary at all.

It's too late to change vm types.  The distinction between addresses
and sizes became useful first with file offsets (vm_ooffset_t.  BTW,
what is an o offset?) and then for physical offsets.

>> Ideally, everything would be in <stdint.h> directly.  I don't know how
>> to do it in a layer, but is it too much to ask for only 2 layers?
>> Hmm, I do know how to do it in 1 layer -- flatten it at install time.
>> Anyone who cared about efficiency and readability would do this :-).
>> But I usually read the uninstalled sources.
>
> If you allow ifdef _LP64 in sys/stdint.h everything in machine/_stdint.h
> could be moved there, except the sig_atomic_t limits which seems to be
> another __clock_t case.

Not sure I like the former.

The current definitions for sig_atomic_t seem to be weird only for
arm (it is long there, but int for all other 32-bit arches including
for the 64 bit half of 32/64-bit ones).  I don't like 64 bits or
even 32 bits for it.  Strictly, a signal handler cannot touch
any global variable except to write to a a sig_atomic_t one.
And sig_atomic_t might be 8 bits.  So portable code can't do much
more than set a single bit in a sig_atomic_t.  So sig_atomic_t
might as well be 8 bits if this is atomic.

Bruce