Page MenuHomeFreeBSD

prison_check(9): Bring up-to-date with hierarchical jails
ClosedPublic

Authored by olce on Jun 20 2023, 1:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 6, 10:43 AM
Unknown Object (File)
Oct 6 2024, 7:38 PM
Unknown Object (File)
Oct 6 2024, 3:44 PM
Unknown Object (File)
Oct 6 2024, 1:44 PM
Unknown Object (File)
Oct 1 2024, 1:34 PM
Unknown Object (File)
Sep 30 2024, 3:47 PM
Unknown Object (File)
Sep 28 2024, 11:21 AM
Unknown Object (File)
Sep 25 2024, 11:01 AM

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

olce requested review of this revision.Jun 20 2023, 1:45 PM

Propose to use active voice in a sentence. The rest looks good.

share/man/man9/prison_check.9
39

Active voice:
This function is used ...
or
This function determines ...

share/man/man9/prison_check.9
33

This is an incomplete sentence, should put the "determine" back as in "determine whether credentials belong..." or something similar.

I think it could also be confusing whether this should be parsed as "belong to (the same jail) or (a sub-jail)" vs "belong to (the same jail or a sub-jail)" although I don't have a great idea for something that is both clear and concise.

share/man/man9/prison_check.9
33

This is an incomplete sentence, should put the "determine" back as in "determine whether credentials belong..." or something similar.

I'll fix the description.

I think it could also be confusing whether this should be parsed as "belong to (the same jail) or (a sub-jail)" vs "belong to (the same jail or a sub-jail)" although I don't have a great idea for something that is both clear and concise.

The two expressions you've quoted are completely equivalent logically, so I'm not sure what you mean.

Re-reading the description, I find unclear which credentials should be in a jail that is a sub-jail of which.

Overall, maybe the best move is to forget about explaining how the checks are done in this one-line description and by contrast focus on the policy's goal. Something like:

checks whether some subject can see an object according to jail confinement

I'll update the diff.

Thanks.

share/man/man9/prison_check.9
33

I mean it could be read as credentials belong to either (case 1) the same jail or (case 2) to a sub-jail, and the function reports which of those two it is.

Your proposed change certainly avoids that. I might say "whether *a* subject" rather than "*some* subject"

Change one-line description. Prefer active tense.

olce marked 3 inline comments as done.Jun 29 2023, 9:17 PM
mhorne added a subscriber: mhorne.
mhorne added inline comments.
share/man/man9/prison_check.9
33

I do not think the description should contain a question. Maybe this?

41

The double-negative should be avoided, although I understand why you chose to word it that way.

Really the function determines both of these realities, so I think it is better described like this.

43–45
olce added inline comments.
share/man/man9/prison_check.9
33

A quick grep through all manual pages shows that currently none exist with a question in the description.

I chose this formulation because I found it more concise and also because it fits on a single line with the function name with standard man page formatting. Also, I find the usual formulations generally more cumbersome. The only drawback to questions I see is that it could appear less formal.

I've switched back to what seems to be the canon of description lines. I hope the latter can evolve and allow questions at some point, as an indication of predicate (or predicate-like) functions.

Done also in all other relevant revisions.

41

The double-negative should be avoided, although I understand why you chose to word it that way.

Two reasons for this wording:

  1. Give a hint that there is an asymmetry between the possible answers in the context of multiple checks. A denial should be treated as final, whereas an acceptance by this function doesn't mean an overall one.
  2. This corresponds better to the return values, in the sense that the behavior described is that of returning 0, the canonical value.

That said, I don't insist since there is a separate RETURN VALUES section, even if I think we are losing a bit of descriptive accuracy by removing this double negation.

Done also in all other relevant revisions.

43–45

You're reversing the flow of thinking here, which I think could be confusing. I'm proposing another, hopefully clearer, formulation.

olce marked 3 inline comments as done.

Address comments.

This revision is now accepted and ready to land.Jul 14 2023, 2:39 PM