Page MenuHomeFreeBSD

sysctl: Add -F option to display sysctl format, refactor for a function to display info options for and added test cases
ClosedPublic

Authored by ota_j.email.ne.jp on Jan 23 2022, 9:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 8, 10:09 PM
Unknown Object (File)
Fri, Mar 8, 9:52 PM
Unknown Object (File)
Fri, Mar 8, 9:52 PM
Unknown Object (File)
Fri, Mar 8, 9:52 PM
Unknown Object (File)
Fri, Mar 8, 9:52 PM
Unknown Object (File)
Fri, Mar 8, 9:48 PM
Unknown Object (File)
Fri, Mar 8, 9:48 PM
Unknown Object (File)
Fri, Mar 8, 9:48 PM

Details

Summary

I'd like to add an option to display fmt.
Before doing so, make a function for displaing info for ease of refactoring.
Added kyua test cases.

Test Plan

% make install, etc...
% kyua test -k /usr/tests/Kyuafile sbin/sysctl

Diff Detail

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

Event Timeline

ota_j.email.ne.jp retitled this revision from Refactor to add a new info option for sysctl and thus added test cases to sysctl: Refactor for a function to display info options for and added test cases.Jan 23 2022, 9:43 PM
ota_j.email.ne.jp added a reviewer: mckusick.
sbin/sysctl/sysctl.c
991

i is assigned but not used. I think it's quite likely that this sysctl call would always succeed as buf is sufficiently large, so if I was you I'd check the return value or maybe convert it to an assertion here.

sbin/sysctl/sysctl.c
1133

Looking at how another sysctl failure is handled, it silently skips printing. I suppose this is for -a, all option.

I will follow the same error handling.

  • Return error code from show_info()
  • Update error description of show_info()
  • systat: Added -F option to show "fmt"
ota_j.email.ne.jp retitled this revision from sysctl: Refactor for a function to display info options for and added test cases to sysctl: Add -F option to display sysctl format, refactor for a function to display info options for and added test cases.Jan 31 2022, 10:59 PM

Have you run mandoc -Tlint and igor on the manual page?

At the very least, the latter would hint that you need to bump .Dd :)

pauamma_gundo.com added inline comments.
sbin/sysctl/sysctl.8
105–106

Also, I'm not sure "structured" is the best word. Maybe "compound"?

  • Bump the date, use "struct" as referenced in other parts of man sysctl, and give few examples of struct types.

Have you run mandoc -Tlint and igor on the manual page?

At the very least, the latter would hint that you need to bump .Dd :)

No, I hadn't known.
Thanks for the info and I had ran.

  • Bump the date, use "struct" as referenced in other parts of man sysctl, and give few examples of struct types.
sbin/sysctl/sysctl.8
105–106

I didn't realize your suggested changes in the first round of changes.

When I looked at lower pages of sysctl.8, I found 'struct' and used it.
You seemed to question about "structured" and didn't pick up this time. If "structured" is a better word, I don't have an object against it.

Manual page language looks good to me. Can't speak about source code or consistency between them.

sbin/sysctl/sysctl.8
105–106

No, "struct" is fine for consistency, I think.

  • sysctl -l to display the lenth of returned data.
bcr added a subscriber: bcr.

Confirm man page looks good.

This all looks good to me. Addition of tests is helpful and appreciated.

This revision is now accepted and ready to land.Oct 16 2022, 2:14 PM

Who's going to commit this?

sbin/sysctl/sysctl.8
116
116

I think "size" (in bytes?) is more accurate than "length".

117
sbin/sysctl/sysctl.c
1006

error is always 0, can we get rid of either error or i?

1068
In D34012#841017, @bcr wrote:

Who's going to commit this?

I am happy to do the commit once it is finalized.

@ota_j.email.ne.jp:
Is this ready to be committed?

Yes, of course. Thank you for pick up the change-set and proceeding.