Date: Sun, 19 Nov 2017 20:07:08 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Maste <emaste@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r325988 - head/sys/libkern Message-ID: <20171119173822.J974@besplex.bde.org> In-Reply-To: <201711190031.vAJ0VE9m016670@repo.freebsd.org> References: <201711190031.vAJ0VE9m016670@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 19 Nov 2017, Ed Maste wrote:
> Log:
> ANSIfy sys/libkern
>
> PR: 223641
> Submitted by: ota@j.email.ne.jp
> MFC after: 1 week
This churns libkern further away from its "vendor" version which is mostly
in libc. I'm still waiting for you to back out previous churning of
kern/md4.c involving "ANSI"fication.
> Modified: head/sys/libkern/ashrdi3.c
The vendor version is in libc/quad. It hasn't been ANSIfied. It ws
identical with the libkern version except for changing the include path
to quad.h and a more gratuitous difference to remove sccsid. The last 2
differences are in most files in libkern and this will not be described
again.
> Modified: head/sys/libkern/bcmp.c
The vendor version is in libc/string. It has been ANSIfied, but the
libkern version has large churning to "optimize" it. It is the libkern
version that should have been optimized, since bcmp is unimportant in
the kernel and in most applications, but applications have a wider range
so a few might benefit from optimizing it. bcmp is actually optimized
in the kernel in support.[sS] for all arches except powerpc and riscv,
so optimizing the kernel MI version of it is especially unimportant.
In libc where MD optimizations are more important, they are also not
done for arm, arm64 and sparc64.
libc bcmp uses simple bytewise comparison. libkern bcmp was optimized
in 1999. The optimizations are only for little endian, so they only
apply to 3/4 of the arches that don't have an MD version (to riscv
but not the big endian part of powerpc). The optimizations are to
compare longwords, and this requires complications for endianness.
> Modified: head/sys/libkern/bsearch.c
The vendor version is in libc/stdlib. It has been ANSIfied. It has an
extra entry point for bsearch_b() and some macros for this. It has the
following gratuitous differences:
- libc version copyright comment not marked for indent protection using
"/*-" (this has been subverted to have another meaning which I forget).
- correct non-difference: libc rccsid not removed. It is ifdefed with
LIBC_SCCSID so it has no effect in the kernel, so removing it in other
places in libkern was churning.
- missing blank line near ids in both and differences in #include's confuse
diff -u into showing identical lines as changed. libc/stdlib sources
mostly have the style bug of not separating the sccsid from _FBSDID().
- COMPAR() macro only used in libc. It is needed for bsearch_b() there,
but is only used once so the macro for it is just an obfuscation.
heapsort.c uses the same macro bt defines its own version with many
more style bugs and uses it 4 times so it is needed there.
- COMPAR() only has the style bug of not having an explicit '*' before
its function pointer dereference in libc
- only the libkern version has not so proper const poisoning for the
assignment to 'base'. At least it doesn't use __DECONST().
- only the libkern version has proper const poisoning for the assignment
to 'base'
The kernel needs const poisoning more since it is compiled at higher
warning levels, but -Wcast-qual is too strict in the kernel too so is
usually (?) not enabled and/or is broken in clang.
> Modified: head/sys/libkern/cmpdi2.c
> ...
> Modified: head/sys/libkern/divdi3.c
> ...
> Modified: head/sys/libkern/lshrdi3.c
Like ashrdi3.c (not ANSIfied in libc/quad).
> Modified: head/sys/libkern/mcount.c
> ==============================================================================
> --- head/sys/libkern/mcount.c Sat Nov 18 21:39:54 2017 (r325987)
> +++ head/sys/libkern/mcount.c Sun Nov 19 00:31:13 2017 (r325988)
> @@ -56,8 +56,7 @@ __FBSDID("$FreeBSD$");
> * both frompcindex and frompc. Any reasonable, modern compiler will
> * perform this optimization.
> */
> -_MCOUNT_DECL(frompc, selfpc) /* _mcount; may be static, inline, etc */
> - uintfptr_t frompc, selfpc;
> +_MCOUNT_DECL(uintfptr_t frompc, uintfptr_t selfpc) /* _mcount; may be static, inline, etc */
The vendor version is in libc/gmon.
The above declaration was already correctly ANSIfied in libc/gmon. This
unimproves the style by keeping the comment misplaced at the right of the
code where it is a larger style bug than before -- not the line is too long.
ANSIfication in libc/gmon put it on a separate line.
> @@ -245,8 +244,7 @@ MCOUNT
>
> #ifdef GUPROF
> void
> -mexitcount(selfpc)
> - uintfptr_t selfpc;
> +mexitcount(uintfptr_t selfpc)
> {
> struct gmonparam *p;
> uintfptr_t selfpcdiff;
This isn't ANSIfied in libc/gmon.
The file still isn't ANSIfied in libkern -- it still has many old-old-style
function definitions near the end for functions taking no args.
The libkern version has nontrivial extensions for the kernel, and a couple
of gratuitous differences in common code. I use a merged version in some
trees. libc should support high resolution profiling too, but still doesn't
even support wide counters for ordinary profiling (its 16-bit counters were
not wide enough 30 years ago. They now overflow in 8 seconds with
profhz = 8192, and profhz = 8192 was not high enough 20 years ago).
> Modified: head/sys/libkern/moddi3.c
Like ashrdi3.c (not ANSIfied in libc/quad).
> Modified: head/sys/libkern/qdivrem.c
Like ashrdi3.c (not ANSIfied in libc/quad), plus:
- only libkern has renamed shl to __shl. This is a static function so doesn't
need renaming except to simplify debugging, and changing it is less needed
where it is changed.
- only libkern has renamed one variable named n to nn. 'n' is a local
variable with a style bug (nested declaration) to give the larger style bug
of shadowing it
> Modified: head/sys/libkern/random.c
The vendor version is in libc/stdlib.
The vendor version is very different -- libkern still uses the 1998 ACM LCG
without even the fixes for that in libc. The libc version has always been
obfuscated by ifdefs and by being in both rand.c and random.c. Together
with a worse comment style in libc, this makes diffs unreadable. Actually,
the worst of the ifdefs has been removed in libc. It was USE_WEAK_SEEDING
for an even worse LCG. libkern never used that.
libc random.c has been fully ANSIfied, but libkern random.c still uses
random() instead of random(void) and libc/stdlib/rand.c still uses
main().
> Modified: head/sys/libkern/scanc.c
There is no vendor version for this.
> Modified: head/sys/libkern/strcmp.c
The vendor version is in libc/string. It has been ANSIfied, but the commit
that did that also made another style fix. This change catches up with half
of the older change.
> Modified: head/sys/libkern/strlcat.c
The vendor version is in libc/string. It has been ANSIfied, and also
C99ified (add 'restrict' -- the kernel mostly hasn't been C99ified yet,
and is missing 'restrict' in most places). It has many other differences,
apparently from updating only it to the NetBSD version in 2015:
- large change to copyright from BSD to Miller (no clauses 1, 2 or 3, and
reformat lots)
- remove rccsid only in libc
- longer variable names
- other changes hard to see due to renaming.
> Modified: head/sys/libkern/strsep.c
The vendor version is in libc/string. It has been ANSIfied. This is about
the only file changed in this commit that doesn't have any gratuitous
differences after the change.
> Modified: head/sys/libkern/strtol.c
The vendor version is in libc/stdio.
The libkern version is missing several fixes:
- special error handling for invalid bases
- fix overflow bugs (?) in overflow checks
- support 'restrict'
and is missing several regressions:
- lossage in copyright
- fix const poisoning using __DECONST()
- uglier locale stuff.
> Modified: head/sys/libkern/strtoul.c
The vendor version is in libc/stdio.
Like strtol.c except fewer fixes and more regressions:
- also lose indent protection in copyright and "From" near copyright
- overflow check was correct, but remove bogus casts in it.
> Modified: head/sys/libkern/ucmpdi2.c
Like ashrdi3.c (not ANSIfied in libc/quad), plus __ARM_EABI__ support
misplaced in this previously MI file, and lexical style bugs in this
support.
> Modified: head/sys/libkern/udivdi3.c
> ...
> Modified: head/sys/libkern/umoddi3.c
Like ashrdi3.c (not ANSIfied in libc/quad).
All of these files were ANSI conformant with fill prototypes in 1995 if
not in 1993.
Many files aren't C99 conformant since they are missing 'restrict'.
I consider this a feature. It reduces churning. Full "ANSI"fication
would also use const a lot to increase churning.
I don't know the exact rules for 'restrict', but it is apparently OK
to only declare functions as 'restrict' in their prototype and omit
this in their function definition. The function definition clearly
doesn't need 'restrict' or even 'const' for the compiler to see what
it does and check this against the prototype, but it is an obfuscation
to use different qualifiers even if the standard allows this.
memcpy() in libc is an example -- it doesn't use 'restrict' in the
implementation. memcpy() in the kernel doesn't even have 'restrict'
in the prototype, and this is not wrong except for compiler bugs
involving using builtin memcpy(), since the kernel is freestanding so
its memcpy() doesn't have to be standard. But the freestanding case
should be even more careful about qualifiers for "standard" functions
since the compiler shouldn't know anything about the functions that
is not in the prototype.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171119173822.J974>