Page MenuHomeFreeBSD

Expose zlib's utility functions in Z_SOLO library when building kernel. This allows kernel code to reuse zlib's implementation.
ClosedPublic

Authored by delphij on Aug 4 2019, 6:18 AM.

Details

Summary

Unlike the gzip file access functions, these functions only require a
working zcalloc/zcfree pair to work.

PR: 229763

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

delphij created this revision.Aug 4 2019, 6:18 AM

I agree with the changes.

When I started, I was also aiming to replace inflate() callers with zlib's GZIP support. Given all of inflate() callers are removed in the process and already, we don't need to deal with std headers. This makes simpler for callers and any ZLIB imports will need to deal with _KERNEL and FreeBSD macros.

sys/contrib/zlib/zconf.h
39 ↗(On Diff #60436)

if !defined(Z_SOLO) && !defined(_KERNEL)

134 ↗(On Diff #60436)
  1. if !defined(Z_SOLO) && !defined(_KERNEL)
sys/dev/zlib/zcalloc.c
31 ↗(On Diff #60436)

I had chosen waitok version in https://reviews.freebsd.org/D20271 based on the number of existing callers. We have more waitok callers than that of nowait.

I'm okay with nowait being default as well.

private email from Xin Li :
Another difference in my proposed change is that zcalloc in my version
would use M_NOWAIT. This is the userland behavior (if memory is full,
the function would bail out instead of block waiting), and I think a
majority of kernel code should be expecting this behavior (especially if
they are holding a lock, for example), and they are expected to handle
errors from zlib anyway.

delphij updated this revision to Diff 60462.Aug 5 2019, 5:49 AM
delphij marked 3 inline comments as done.

Address reviewer comments.

Please take another look.

sys/modules/zlib/Makefile
21 ↗(On Diff #60462)

S is missing - SRCS

delphij marked an inline comment as done.Aug 6 2019, 5:20 AM

Good catch!

delphij updated this revision to Diff 60490.Aug 6 2019, 5:20 AM

Fix typo.

I found and have one more minor comment.

The changes look good.
I suppose one of FreeBSD committer needs to accept this review.

sys/conf/files
4007 ↗(On Diff #60490)

This is a bit picky one.
Given sys/module/zlib/Makefile doesn't pass -Wno-cast-qual to crc32, we may not need this here, neither.

delphij updated this revision to Diff 60493.Aug 6 2019, 6:30 AM

Fix compile options for zlib crc32.c.

delphij marked an inline comment as done.Aug 6 2019, 6:31 AM

Thanks! I think I'll commit this at some point this week (doing a tinderbox build now).

This revision was not accepted when it landed; it landed in state Needs Review.Aug 7 2019, 1:41 AM
This revision was automatically updated to reflect the committed changes.