Page MenuHomeFreeBSD
Differential D21156

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

Authored by delphij on Aug 4 2019, 6:18 AM.
Tags
None
Referenced Files
F114725358: D21156.diff
Tue, Apr 15, 11:44 PM
Unknown Object (File)
Tue, Apr 15, 3:53 AM
Unknown Object (File)
Sun, Apr 13, 8:01 AM
Unknown Object (File)
Wed, Apr 2, 8:09 PM
Unknown Object (File)
Wed, Mar 26, 12:00 PM
Unknown Object (File)
Mar 14 2025, 4:54 PM
Unknown Object (File)
Mar 6 2025, 2:15 PM
Unknown Object (File)
Mar 5 2025, 11:36 PM
View All Files
Subscribers
imp

Details

Reviewers
emaste
ota_j.email.ne.jp
Commits
rS350670: Expose zlib's utility functions in Z_SOLO library when building kernel.
Summary

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

PR: 229763

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 25686
Build 24275: arc lint + arc unit

Event Timeline

delphij created this revision.Aug 4 2019, 6:18 AM
Harbormaster completed remote builds in B25669: Diff 60436.Aug 4 2019, 6:18 AM
ota_j.email.ne.jp added a comment.Aug 4 2019, 3:34 PM

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

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

134
  1. if !defined(Z_SOLO) && !defined(_KERNEL)
sys/dev/zlib/zcalloc.c
31

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.

ota_j.email.ne.jp added a child revision: D20271: geom_uzip to update zlib version 1.0.4 to 1.2.11..Aug 4 2019, 3:37 PM
delphij updated this revision to Diff 60462.Aug 5 2019, 5:49 AM
delphij marked 3 inline comments as done.

Address reviewer comments.

Harbormaster completed remote builds in B25686: Diff 60462.Aug 5 2019, 5:49 AM
delphij added a comment.Aug 5 2019, 5:50 AM

Please take another look.

ota_j.email.ne.jp added inline comments.Aug 5 2019, 10:55 PM
sys/modules/zlib/Makefile
21

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.

Harbormaster completed remote builds in B25692: Diff 60490.Aug 6 2019, 5:20 AM
ota_j.email.ne.jp added a comment.Aug 6 2019, 5:43 AM

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
4006

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.

Harbormaster completed remote builds in B25695: Diff 60493.Aug 6 2019, 6:30 AM
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
Closed by commit rS350670: Expose zlib's utility functions in Z_SOLO library when building kernel. (authored by delphij). · Explain Why
This revision was automatically updated to reflect the committed changes.
delphij added a commit: rS350670: Expose zlib's utility functions in Z_SOLO library when building kernel..
Herald added a subscriber: imp. · View Herald TranscriptAug 7 2019, 1:41 AM

Revision Contents
Changeset List

PathSize
sys/
conf/
38 lines
4 lines
2 lines
contrib/
zlib/
4 lines
4 lines
4 lines
12 lines
5 lines
2 lines
dev/
zlib/
7 lines
7 lines
modules/
crypto/
1 line
mxge/
mxge/
2 lines
zfs/
1 line
zlib/
4 lines

Diff 60462

View Options

sys/conf/files

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Linesprocfile(const char *fn)
for (c = 0; c == 0 || !(lflag || qflag); ) { for (c = 0; c == 0 || !(lflag || qflag); ) {
ln.off += ln.len + 1; ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL || ln.len == 0) { if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL || ln.len == 0) {
if (ln.line_no == 0 && matchall) if (ln.line_no == 0 && matchall)
exit(0); exit(0);
else else
break; break;
} }
if (ln.len > 0 && ln.dat[ln.len - 1] == '\n') if (ln.len > 0 && ln.dat[ln.len - 1] == fileeol)
--ln.len; --ln.len;
ln.line_no++; ln.line_no++;
/* Return if we need to skip a binary file */ /* Return if we need to skip a binary file */
if (f->binary && binbehave == BINFILE_SKIP) { if (f->binary && binbehave == BINFILE_SKIP) {
grep_close(f); grep_close(f);
free(ln.file); free(ln.file);
free(f); free(f);
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines if ((oflag || color) && m > 0) {
} }
if (!oflag) { if (!oflag) {
if (line->len - a > 0) if (line->len - a > 0)
fwrite(line->dat + a, line->len - a, 1, stdout); fwrite(line->dat + a, line->len - a, 1, stdout);
putchar('\n'); putchar('\n');
} }
} else { } else {
fwrite(line->dat, line->len, 1, stdout); fwrite(line->dat, line->len, 1, stdout);
putchar('\n'); putchar(fileeol);
cemUnsubmitted
Not Done
Inline Actions

Hm, should the nul input separater really be used for outputting matched lines?

And if so, shouldn't it also be used with oflag above?

cem: Hm, should the nul input separater really be used for outputting matched lines? And if so…
kevansAuthorUnsubmitted
Not Done
Inline Actions

So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful to the input file -- i.e., this line, and actually line 491 that I missed (!oflag, but color). Everything else is printing excerpts and should use the standard grep newline separator.

kevans: So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful…
} }
} }
View Options

sys/conf/kern.pre.mk

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Linesprocfile(const char *fn)
for (c = 0; c == 0 || !(lflag || qflag); ) { for (c = 0; c == 0 || !(lflag || qflag); ) {
ln.off += ln.len + 1; ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL || ln.len == 0) { if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL || ln.len == 0) {
if (ln.line_no == 0 && matchall) if (ln.line_no == 0 && matchall)
exit(0); exit(0);
else else
break; break;
} }
if (ln.len > 0 && ln.dat[ln.len - 1] == '\n') if (ln.len > 0 && ln.dat[ln.len - 1] == fileeol)
--ln.len; --ln.len;
ln.line_no++; ln.line_no++;
/* Return if we need to skip a binary file */ /* Return if we need to skip a binary file */
if (f->binary && binbehave == BINFILE_SKIP) { if (f->binary && binbehave == BINFILE_SKIP) {
grep_close(f); grep_close(f);
free(ln.file); free(ln.file);
free(f); free(f);
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines if ((oflag || color) && m > 0) {
} }
if (!oflag) { if (!oflag) {
if (line->len - a > 0) if (line->len - a > 0)
fwrite(line->dat + a, line->len - a, 1, stdout); fwrite(line->dat + a, line->len - a, 1, stdout);
putchar('\n'); putchar('\n');
} }
} else { } else {
fwrite(line->dat, line->len, 1, stdout); fwrite(line->dat, line->len, 1, stdout);
putchar('\n'); putchar(fileeol);
cemUnsubmitted
Not Done
Inline Actions

Hm, should the nul input separater really be used for outputting matched lines?

And if so, shouldn't it also be used with oflag above?

cem: Hm, should the nul input separater really be used for outputting matched lines? And if so…
kevansAuthorUnsubmitted
Not Done
Inline Actions

So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful to the input file -- i.e., this line, and actually line 491 that I missed (!oflag, but color). Everything else is printing excerpts and should use the standard grep newline separator.

kevans: So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful…
} }
} }
View Options

sys/conf/kmod.mk

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Linesprocfile(const char *fn)
for (c = 0; c == 0 || !(lflag || qflag); ) { for (c = 0; c == 0 || !(lflag || qflag); ) {
ln.off += ln.len + 1; ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL || ln.len == 0) { if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL || ln.len == 0) {
if (ln.line_no == 0 && matchall) if (ln.line_no == 0 && matchall)
exit(0); exit(0);
else else
break; break;
} }
if (ln.len > 0 && ln.dat[ln.len - 1] == '\n') if (ln.len > 0 && ln.dat[ln.len - 1] == fileeol)
--ln.len; --ln.len;
ln.line_no++; ln.line_no++;
/* Return if we need to skip a binary file */ /* Return if we need to skip a binary file */
if (f->binary && binbehave == BINFILE_SKIP) { if (f->binary && binbehave == BINFILE_SKIP) {
grep_close(f); grep_close(f);
free(ln.file); free(ln.file);
free(f); free(f);
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines if ((oflag || color) && m > 0) {
} }
if (!oflag) { if (!oflag) {
if (line->len - a > 0) if (line->len - a > 0)
fwrite(line->dat + a, line->len - a, 1, stdout); fwrite(line->dat + a, line->len - a, 1, stdout);
putchar('\n'); putchar('\n');
} }
} else { } else {
fwrite(line->dat, line->len, 1, stdout); fwrite(line->dat, line->len, 1, stdout);
putchar('\n'); putchar(fileeol);
cemUnsubmitted
Not Done
Inline Actions

Hm, should the nul input separater really be used for outputting matched lines?

And if so, shouldn't it also be used with oflag above?

cem: Hm, should the nul input separater really be used for outputting matched lines? And if so…
kevansAuthorUnsubmitted
Not Done
Inline Actions

So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful to the input file -- i.e., this line, and actually line 491 that I missed (!oflag, but color). Everything else is printing excerpts and should use the standard grep newline separator.

kevans: So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful…
} }
} }
View Options

sys/contrib/zlib/deflate.c

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Linesprocfile(const char *fn)
for (c = 0; c == 0 || !(lflag || qflag); ) { for (c = 0; c == 0 || !(lflag || qflag); ) {
ln.off += ln.len + 1; ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL || ln.len == 0) { if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL || ln.len == 0) {
if (ln.line_no == 0 && matchall) if (ln.line_no == 0 && matchall)
exit(0); exit(0);
else else
break; break;
} }
if (ln.len > 0 && ln.dat[ln.len - 1] == '\n') if (ln.len > 0 && ln.dat[ln.len - 1] == fileeol)
--ln.len; --ln.len;
ln.line_no++; ln.line_no++;
/* Return if we need to skip a binary file */ /* Return if we need to skip a binary file */
if (f->binary && binbehave == BINFILE_SKIP) { if (f->binary && binbehave == BINFILE_SKIP) {
grep_close(f); grep_close(f);
free(ln.file); free(ln.file);
free(f); free(f);
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines if ((oflag || color) && m > 0) {
} }
if (!oflag) { if (!oflag) {
if (line->len - a > 0) if (line->len - a > 0)
fwrite(line->dat + a, line->len - a, 1, stdout); fwrite(line->dat + a, line->len - a, 1, stdout);
putchar('\n'); putchar('\n');
} }
} else { } else {
fwrite(line->dat, line->len, 1, stdout); fwrite(line->dat, line->len, 1, stdout);
putchar('\n'); putchar(fileeol);
cemUnsubmitted
Not Done
Inline Actions

Hm, should the nul input separater really be used for outputting matched lines?

And if so, shouldn't it also be used with oflag above?

cem: Hm, should the nul input separater really be used for outputting matched lines? And if so…
kevansAuthorUnsubmitted
Not Done
Inline Actions

So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful to the input file -- i.e., this line, and actually line 491 that I missed (!oflag, but color). Everything else is printing excerpts and should use the standard grep newline separator.

kevans: So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful…
} }
} }
View Options

sys/contrib/zlib/infback.c

Loading...
View Options

sys/contrib/zlib/inflate.c

Loading...
View Options

sys/contrib/zlib/zconf.h

Loading...
View Options

sys/contrib/zlib/zlib.h

Loading...
View Options

sys/contrib/zlib/zutil.h

Loading...
View Options

sys/dev/zlib/zcalloc.h

Loading...
View Options

sys/dev/zlib/zcalloc.c

Loading...
View Options

sys/modules/crypto/Makefile

Loading...
View Options

sys/modules/mxge/mxge/Makefile

Loading...
View Options

sys/modules/zfs/Makefile

Loading...
View Options

sys/modules/zlib/Makefile

Loading...