Page MenuHomeFreeBSD

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

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



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

PR: 229763

Diff Detail

rS FreeBSD src repository - subversion
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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.

39 ↗(On Diff #60436)

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

134 ↗(On Diff #60436)
  1. if !defined(Z_SOLO) && !defined(_KERNEL)
31 ↗(On Diff #60436)

I had chosen waitok version in 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 marked 3 inline comments as done.

Address reviewer comments.

21 ↗(On Diff #60462)

S is missing - SRCS

I found and have one more minor comment.

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

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.

Fix compile options for zlib crc32.c.

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.