Details

Reviewers

kib
jmg
ota_j.email.ne.jp
delphij

Commits

rS349151: Separate kernel crc32() implementation to its own header (gsb_crc32.h) and

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 Passed

Unit

No Test Coverage

Build Status

Buildable 24272
Build 23099: arc lint + arc unit

Event Timeline

ota_j.email.ne.jp created this revision.May 8 2019, 12:30 AM

Herald added subscribers: Contributor Reviews (src), imp. · View Herald TranscriptMay 8 2019, 12:30 AM

Harbormaster completed remote builds in B24146: Diff 57154.May 8 2019, 12:30 AM

Combine SCTP cases.

Harbormaster completed remote builds in B24149: Diff 57161.May 8 2019, 2:10 AM

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
62	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
41	Should this extern declaration conditionalized for _KERNEL ? Is this header supposed to be useful for usermode at all ?

Fixed function prototype style.

Harbormaster completed remote builds in B24166: Diff 57205.May 9 2019, 12:21 AM

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

Harbormaster completed remote builds in B24212: Diff 57337.May 12 2019, 2:51 PM

ota_j.email.ne.jp mentioned this in D19706: Allow Kernel to link in both legacy libkern/zlib and new sys/contrib/zlib..May 14 2019, 3:06 AM

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

ota_j.email.ne.jp added a child revision: D20271: geom_uzip to update zlib version 1.0.4 to 1.2.11..May 16 2019, 4:13 AM

ota_j.email.ne.jp added a child revision: D20272: if_mxge to update zlib version 1.0.4 to 1.2.11..May 16 2019, 4:34 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.

Harbormaster completed remote builds in B24270: Diff 57467.May 17 2019, 1:57 AM

Remove one more extra prototype.

Harbormaster completed remote builds in B24271: Diff 57468.May 17 2019, 2:04 AM

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

Harbormaster completed remote builds in B24272: Diff 57469.May 17 2019, 2:51 AM

ota_j.email.ne.jp marked an inline comment as done.May 18 2019, 3:48 PM

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

Svn up and resolved conflicts.

Harbormaster completed remote builds in B24342: Diff 57612.May 20 2019, 10:16 PM

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.

Harbormaster completed remote builds in B24575: Diff 58016.May 29 2019, 12:34 AM

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)

Fix KERN to GSB.

Harbormaster completed remote builds in B24576: Diff 58017.May 29 2019, 1:05 AM

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 commandeered this revision.Jun 19 2019, 2:37 PM

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

Committed as rS349151

delphij added a commit: rS349151: Separate kernel crc32() implementation to its own header (gsb_crc32.h) and.Jul 6 2019, 9:21 AM

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

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 57469

sys/conf/files

sys/dev/iscsi/icl_soft.c

sys/dev/iscsi_initiator/isc_subr.c

sys/dev/liquidio/lio_bsd.h

sys/dev/usb/net/if_cdce.c

sys/fs/ext2fs/ext2_csum.c

sys/geom/part/g_part_bsd64.c

sys/geom/part/g_part_gpt.c

sys/geom/raid/md_ddf.c

sys/kern/subr_compressor.c

sys/libkern/crc32.c

sys/libkern/kern_crc32.c

sys/libkern/x86/crc32_sse42.c

sys/netinet/libalias/alias_sctp.c

sys/netinet/sctp_crc32.c

sys/sys/crc32.h

sys/sys/libkern.h

sys/ufs/ffs/ffs_alloc.c

sys/ufs/ffs/ffs_snapshot.c

sys/ufs/ffs/ffs_subr.c

sys/ufs/ffs/ffs_vfsops.c

tests/sys/kern/Makefile

tests/sys/kern/libkern_crc32.c

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 57469

sys/conf/files

sys/dev/iscsi/icl_soft.c

sys/dev/iscsi_initiator/isc_subr.c

sys/dev/liquidio/lio_bsd.h

sys/dev/usb/net/if_cdce.c

sys/fs/ext2fs/ext2_csum.c

sys/geom/part/g_part_bsd64.c

sys/geom/part/g_part_gpt.c

sys/geom/raid/md_ddf.c

sys/kern/subr_compressor.c

sys/libkern/crc32.c

sys/libkern/kern_crc32.c

sys/libkern/x86/crc32_sse42.c

sys/netinet/libalias/alias_sctp.c

sys/netinet/sctp_crc32.c

sys/sys/crc32.h

sys/sys/libkern.h

sys/ufs/ffs/ffs_alloc.c

sys/ufs/ffs/ffs_snapshot.c

sys/ufs/ffs/ffs_subr.c

sys/ufs/ffs/ffs_vfsops.c

tests/sys/kern/Makefile

tests/sys/kern/libkern_crc32.c

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

Revision Contents
Changeset List