Page MenuHomeFreeBSD

Relocate crc32 functions from libkern.h to gsb_crc32.h/c
ClosedPublic

Authored by delphij on May 8 2019, 12:30 AM.

Details

Summary

This is to avoid API conflict of crc32() with zlib.h.
ABI conflict can be avoided with -DZ_PREFIX but that requires header file split.
Use and honor Gary S. Brown as filename prefix.

Test Plan

No functionality changes are made.
% make buildworld buildkernel TARGET_ARCH=<arch>
% make installkernel
... reboot etc.
% make installworld
... reboot etc
% make -C .../tests/sys/kern
with nooptions SCTP and options SCTP for i386, amd64 and aach64/arm64

And some/few regression tests.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 24576
Build 23362: arc lint + arc unit

Event Timeline

Combine SCTP cases.

ota_j.email.ne.jp edited the test plan for this revision. (Show Details)May 8 2019, 2:12 AM
ota_j.email.ne.jp added reviewers: kib, delphij, jmg.
kib added inline comments.May 8 2019, 9:11 AM
sys/sys/crc32.h
61 ↗(On Diff #57161)

There should be no line split in the function declaration after the return type.

kib added inline comments.May 8 2019, 9:12 AM
sys/sys/crc32.h
40 ↗(On Diff #57161)

Should this extern declaration conditionalized for _KERNEL ? Is this header supposed to be useful for usermode at all ?

ota_j.email.ne.jp edited the test plan for this revision. (Show Details)May 9 2019, 12:20 AM
ota_j.email.ne.jp updated this revision to Diff 57205.

Fixed function prototype style.

ota_j.email.ne.jp retitled this revision from Split crc32 functions libkern.h to crc32.h to Relocate crc32 functtions from libkern.h to crc32.h.May 9 2019, 4:33 AM
ota_j.email.ne.jp edited the test plan for this revision. (Show Details)
ota_j.email.ne.jp marked an inline comment as done.May 9 2019, 4:37 AM
ota_j.email.ne.jp added inline comments.
sys/sys/crc32.h
40 ↗(On Diff #57161)

crc32_tab[] is used in crc32_raw() below.

Do you mean to surround all of crc32 functions with ifdef _KERNEL?
crc32.c doesn't have _KERNEL.
It looks easy to compile with sys/crc32.h header but there is no library created for user land usage and also some architectures have assembly code as well. I don't think linking with kernel crc32 is easy.

zlib provides crc32() function as well and easy to link with -lz.

ota_j.email.ne.jp planned changes to this revision.May 12 2019, 4:02 AM

I'd like to rename libkern/crc32.c to libkern/kern_crc32.c so that we don't get object filename conflict with zlib/crc32.c as well.
I'm having difficulties with arc diff...

Rename libkern/crc32.c to libkern/kern_crc32.c to avoid having same object
filename as zlib/crc32.c.

I wonder if we'd better rename the header to sys/kern_crc32.h as well.

ota_j.email.ne.jp retitled this revision from Relocate crc32 functtions from libkern.h to crc32.h to Relocate crc32 functions from libkern.h to crc32.h and rename crc32.c to kern_crc32.c.May 14 2019, 3:07 AM
delphij added inline comments.May 16 2019, 4:55 PM
sys/libkern/x86/crc32_sse42.c
32

Unrelated to your change, but maybe we can change this to #if(n)def _KERNEL (along with the comment above) and simply drop USERSPACE_TESTING in tests/sys/kern/Makefile?

Replace ifdef USERSPACE_TESTING to ifndef _KERNEL.

Remove one more extra prototype.

ota_j.email.ne.jp marked an inline comment as done.May 17 2019, 2:15 AM
ota_j.email.ne.jp added inline comments.
sys/libkern/x86/crc32_sse42.c
32

Now, I get kib's _KERNEL question at last.

I adjusted and ran 'make -c tests/sys/kern' to test.

ota_j.email.ne.jp marked an inline comment as done.May 17 2019, 2:17 AM
ota_j.email.ne.jp added inline comments.
sys/libkern/crc32.c
1

By the way, there are files named sys/kern/kern_xxx.c and sys/libkern/xxx.c.
sys/libkern/kern_crc32.c format doesn't follow any of existing filename format.

What's the policy between kern and libkern directory?

ota_j.email.ne.jp edited the test plan for this revision. (Show Details)May 17 2019, 2:19 AM

More _KERNEL adjustments.

ota_j.email.ne.jp marked an inline comment as done.May 18 2019, 3:48 PM
ota_j.email.ne.jp planned changes to this revision.May 20 2019, 5:09 AM

r347984 creates conflicts for lio_bsd.h and if_cdce.c.
I need to svn up and rebase.

Svn up and resolved conflicts.

After research, I found that our crc32 functions are based on the code written
by Gary S Brown. We also have many other *crc32.[hc] files, too. Rather than,
prefixing with kern, prefix with his initial to honor his work.
It looks kern prefix is used for sub-systems.

ota_j.email.ne.jp retitled this revision from Relocate crc32 functions from libkern.h to crc32.h and rename crc32.c to kern_crc32.c to Relocate crc32 functions from libkern.h to gsb_crc32.h/c.Wed, May 29, 12:47 AM
ota_j.email.ne.jp edited the summary of this revision. (Show Details)

Fix KERN to GSB.

Harbormaster completed remote builds in B24576: Diff 58017.

Xin committed this change with minor adjustments and https://svnweb.freebsd.org/base?view=revision&revision=349151 was the revision.

delphij edited reviewers, added: ota_j.email.ne.jp; removed: delphij.Wed, Jun 19, 2:37 PM
delphij commandeered this revision.
delphij accepted this revision.
This revision is now accepted and ready to land.Wed, Jun 19, 2:37 PM
delphij closed this revision.Wed, Jun 19, 2:37 PM

Committed as rS349151