Page MenuHomeFreeBSD

save-entropy.sh: Add manual page
ClosedPublic

Authored by fernape on Sep 7 2023, 8:33 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 29, 12:38 AM
Unknown Object (File)
Fri, Jun 28, 10:20 PM
Unknown Object (File)
Thu, Jun 13, 11:57 PM
Unknown Object (File)
Wed, Jun 5, 1:21 PM
Unknown Object (File)
May 18 2024, 6:17 AM
Unknown Object (File)
May 11 2024, 5:16 PM
Unknown Object (File)
May 6 2024, 8:15 AM
Unknown Object (File)
May 5 2024, 1:37 PM

Details

Summary

PR: 223998
Reported by: AJ Jordan <alex+freebsd@strugee.net>

Test Plan
  • Apply patch
  • cd libexec/save-entropy && make
  • man ./save-entropy.sh.8
  • mandoc -T lint, igor and vale outputs clean

Diff Detail

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

Event Timeline

It is better to put in section 8 and named as save-entropy.8 as it's installed as /usr/libexec/save-entropy ?

  • Move to section 8 as suggested by lwhsu

I also suggested that trim the .sh suffix as it's installed as /usr/libexec/save-entropy.

  • s/save-entropy.sh/save-entropy/

    We drop the .sh at install time.

I also suggested that trim the .sh suffix as it's installed as /usr/libexec/save-entropy.

Sorry, I just realized after updating the review.

libexec/save-entropy/save-entropy.sh.8
2 ↗(On Diff #127049)

Now we use BSD-2-Clause

Ref: rG4d846d260e2b9a3d4d0a701462568268cbfe7a5b

libexec/save-entropy/save-entropy.sh.8
2 ↗(On Diff #127049)

I wasn't aware of that. We have a mix of things in src regarding to license.
Before I change that, is it sufficient with the SPDX identifier?
We have files with the identifier plus the verbatim text of the license, other with only the identifier...

libexec/save-entropy/save-entropy.sh.8
2 ↗(On Diff #127049)

I think at least you need your name in the copyright announcement. For this kind of single file as a whole, I would say providing the verbatim is fine. Just be sure you copy-pasted from the correct template.

fernape added inline comments.
libexec/save-entropy/save-entropy.sh.8
2 ↗(On Diff #127049)

I think at least you need your name in the copyright announcement. For this kind of single file as a whole, I would say providing the verbatim is fine. Just be sure you copy-pasted from the correct template.

Thanks.

I change the SPDX ID. The text was already that from the 2-Clause BSD License as copied from https://opensource.org/license/bsd-2-clause/

libexec/save-entropy/save-entropy.8
43

how about putting a period symbol to the end of the sentence:

.Pa /dev/random .

45

how about s/typically/by default/ ?

57

A period symbol to the end of the sentence?

.Xr cron 8 .

82

Not sure if we want to add

.Xr rc.conf 5 ,

as entropy_s* also described there.

fernape marked an inline comment as done.

Address feedback:

  • Rewording
  • Add full stops where needed
  • Add Xr to rc.conf
  • Use macro for standard exit status

Thanks very much!

BTW, as a doc committer, you don't actually need src committer's approval to commit to manual pages in the src repository.

This revision is now accepted and ready to land.Sep 8 2023, 7:04 AM

Thanks very much!

Thank you!

BTW, as a doc committer, you don't actually need src committer's approval to commit to manual pages in the src repository.

I know, but it is always good to have an extra pair of eyes. The final version is much better than the first one.

It's probably worth mentioning that the script will not do anything in a jail (because it doesn't provide much value).

libexec/save-entropy/save-entropy.8
33

Nit: these are used for "startup" (rc(8)); boot-time entropy file (loader.conf(8) entropy_cache_load and entropy_cache_name) are only updated one time at startup and not touched by save-entropy.

42
43
53

Minor nit: Technically entropy_dir is used by /etc/rc.d/random too, so we should probably say:

Specify the directory for saved entropy files.
pauamma_gundo.com added inline comments.
libexec/save-entropy/save-entropy.8
42

Why "files" here when the previous line mentions a single file? (If "files" is correct as hinted later on, perhaps "Several files may be saved to in successive invocations; all are used...")

libexec/save-entropy/save-entropy.8
42

Why "files" here when the previous line mentions a single file? (If "files" is correct as hinted later on, perhaps "Several files may be saved to in successive invocations; all are used...")

It should be "files" and the line above should probably say "to a specified location" instead.

  • save-entropy: Add manual page
  • Address feedback.
This revision now requires review to proceed.Sep 17 2023, 5:10 PM

Thank you for all the feedback.

libexec/save-entropy/save-entropy.8
53

I also noted that this is a shared setting.

This revision is now accepted and ready to land.Sep 17 2023, 10:39 PM
This revision was automatically updated to reflect the committed changes.
fernape marked an inline comment as done.