Page MenuHomeFreeBSD

Introduce sys/kassert.h
ClosedPublic

Authored by kib on Jan 29 2022, 4:06 AM.

Details

Summary
It contains assert-related definitions previously provided by
sys/systm.h.  The new header is leaner than whole systm.h.

Include kassert.h from systm.h for compatibility.
vm/vm_extern.h: use sys/kassert.h

instead of fatty sys/systm.h.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

kib requested review of this revision.Jan 29 2022, 4:06 AM

I copied license from systm.h, but clearly UCB has no relations to this relatively modern stuff at all. Not sure what to do there.

sys/sys/kassert.h
115

Should these also be _STANDALONE, except for the SCHEDULER_STOPPED_TD()?

sys/sys/kassert.h
115

I built stand/ with the patch applied, and it passed (I did not yet tinderboxed the patch).

In principle MPASSX() can be useful in stand/, but it seems to be unused. Do you want to have them enabled there, I do not believe that they are right now?

In D34089#770782, @kib wrote:

I copied license from systm.h, but clearly UCB has no relations to this relatively modern stuff at all. Not sure what to do there.

There is only one line copied verbatim from UCB systm.h:

extern const char *panicstr; /* panic message */

and two lines with similar content:

void panic(const char *, ...) dead2 printflike(1, 2);
int printf(const char *, ...) __printflike(1, 2);

I believe that this file should get a new copyright which I would date 1999 since that is when most of what is carried over here was first added to it (see 5526d2d920eb17b150).

In D34089#770782, @kib wrote:

I copied license from systm.h, but clearly UCB has no relations to this relatively modern stuff at all. Not sure what to do there.

There is only one line copied verbatim from UCB systm.h:

extern const char *panicstr; /* panic message */

and two lines with similar content:

void panic(const char *, ...) dead2 printflike(1, 2);
int printf(const char *, ...) __printflike(1, 2);

I believe that this file should get a new copyright which I would date 1999 since that is when most of what is carried over here was first added to it (see 5526d2d920eb17b150).

Am I right that you are suggesting to assign copyright to Eivind Eklund <eivind@FreeBSD.org> ?
Thank you for the investigation.

Eivind Eklund was the person that first added KASSERT which was the start of what probably should have been called kassert.h. Many others added to it over time probably putting more effort into than he did. But I am inclined to give him credit for starting the ball rolling. He was an active committer from February 1997 until June 2005 (461 base, 69 ports, 26 doc). I note that he still has a freefall account.

Assign sys/kassert.h copyright to eivind, as suggested by mckusick. The 3 clause BSD license is kept for now, might be changed if eivind reply [he does not have phab account].

git blame shows maybe 10 people write this code, maybe more if we look at the whole history.
Eivind did start the ball rolling, but little of his original code remains. I may have the biggest block,
but even that code was copied, greatly inspired by earlier efforts and shows a disproportionate
weighting towards me. Then gleb and mjg are the next in terms of raw lines, with Attilio and maybe
phk in the next tier, though kyle, alfred, matt, peter and a few others show signs of having made
changes along the way, some extensive. And some of this code was copied from elsewhere in the
tree along the way, obscuring things a bit further.

tl;dr: There are no good choices here, but Eivind is likely the least bad as the initial mover.

sys/sys/kassert.h
115

I'm torn, but I think leaving at it is might be a tiny bit better.
stand uses stuff from the kernel, and these are in the kernel.
But they grew up in mostly the network side of the kernel, which stand borrows nothing from.
So I'll sign on on this as is and we'll expand if/when new code needs it.

Modulo the copyright issue, this looks good to go.

This revision is now accepted and ready to land.Jan 31 2022, 4:47 PM
jhb added inline comments.
sys/vm/vm_extern.h
45

I would suggest also trying this in vm/vm_page.h which also has a nested include of sys/systm.h?

kib marked an inline comment as done.Feb 1 2022, 4:09 AM
kib added inline comments.
sys/vm/vm_extern.h
45

vm/vm_page.h inclusion of sys/systm.h was abused at least by vm/vm_pager.h. I changed the later to directly include systm.h.

This revision was automatically updated to reflect the committed changes.
kib marked an inline comment as done.