Page MenuHomeFreeBSD

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

Authored by delphij on May 8 2019, 12:30 AM.
Referenced Files
Unknown Object (File)
Sun, Dec 4, 9:49 AM
Unknown Object (File)
Tue, Nov 29, 6:31 AM
Unknown Object (File)
Mon, Nov 28, 1:47 AM



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

rS FreeBSD src repository - subversion
Lint Passed
No Test Coverage
Build Status
Buildable 24576
Build 23362: arc lint + arc unit

Event Timeline

61 ↗(On Diff #57161)

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

40 ↗(On Diff #57161)

Should this extern declaration conditionalized for _KERNEL ? Is this header supposed to be useful for usermode at all ? edited the test plan for this revision. (Show Details)

Fixed function prototype style. 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 edited the test plan for this revision. (Show Details) added inline comments.
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. 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

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. added inline comments.

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

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

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