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 - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 24576
Build 23362: arc lint + arc unit

Event Timeline

sys/sys/crc32.h
61 ↗(On Diff #57161)

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

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)

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 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.

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
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.

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 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?

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

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.May 29 2019, 12:47 AM
ota_j.email.ne.jp edited the summary of this revision. (Show Details)
delphij accepted this revision.
delphij edited reviewers, added: ota_j.email.ne.jp; removed: delphij.
This revision is now accepted and ready to land.Jun 19 2019, 2:37 PM