sysctlmibinfo(3)

[asiciliano]

Summary

The sysctlmibinfo library is an interface to the kernel sysctl-mib-tree.
It implements wrappers around undocumented "sysctl.*" kernel states to get mib-entries information and provides a convenient API to build a mib-entry, entries-list and mib-tree in userspace.

The advantages to have sysctlmibinfo are:

• an easy API to the kernel sysctl mib-tree,
• a handy mib-entry implementation,
• building entries-list and mib-tree in userspace,
• writing quickly a custom sysctl(8) tool,
• changes to kern_sysctl.c interface won't upset userspace tools.

Test Plan

Files *.c in examples/ show every function

• examples.

Tools using libsysctl

• "sysctlview 0.1", a sysctl gtk tool
• "nsysctl 0.1", sysctl(8) + [-FIlMmSy] + [--libxo]

[bcr]

Thanks for your work! I found one issue with the man page.
Can you run "mandoc -Tlint" and the textproc/igor package over the man page and see if additional things are reported?

lib/libsysctl/libsysctl.3

113 ↗

(On Diff #52443)

There must be a line break after a sentence stop.

[asiciliano]

lib/libsysctl/libsysctl.3

113 ↗

(On Diff #52443)

Thank you for your suggestion, I'm updating to fix "mandoc -Tlint" and "igor -Dd"

[brooks]

This looks like a neat library. I've made a few comments based on a casual review.

lib/libsysctl/libsysctl.3

293 ↗

(On Diff #52443)

I think I'd just talk about allocated memory rather than heap.

lib/libsysctl/libsysctl.c

90 ↗

(On Diff #52443)

Casting to void* before passing as a void* argument seems unnecessary.

171 ↗

(On Diff #52443)

This feels like it should be done in a temporary with *idnextlen only updated on success (or perhaps unconditionally zeroed).

226 ↗

(On Diff #52443)

No need to cast malloc().

241 ↗

(On Diff #52443)

These are redundant to the bzero.

243 ↗

(On Diff #52443)

Using memset() would allow the compiler to elide much of the zeroing.

255 ↗

(On Diff #52443)

This leaks memory as do all the other malloc failure paths in this function.

315 ↗

(On Diff #52443)

looks like too much indentation for style(9) (but could be phabricator).

448 ↗

(On Diff #52443)

Declaration should be at the top of the function.

lib/libsysctl/libsysctl.h

44 ↗

(On Diff #52443)

I'd find the headers a lot easier to read if you included variable names in the function prototypes.

79 ↗

(On Diff #52443)

While this layout (pairing length and pointer) makes the structure easy to read, it's somewhat expensive on architectures where pointers are larger and have stronger alignment constraints than size_t. E.g. on 128-bit CHERI this layout would waste an extra 32-bytes on padding which could add up if you have a lot of these.

90 ↗

(On Diff #52443)

I'd say children here.

[bapt]

why not adding that to libutils instead of adding another library?

[asiciliano]

This comment was removed by asiciliano.

[asiciliano]

Thanks you all for the comments,
I updated to fix, soon the "update diff".

In D18696#400437, @bapt wrote:

why not adding that to libutils instead of adding another library?

I used "libsysctl_" prefix to avoid naming conflict from <sys/sysctl.h>. Moreover a programmer easily distinguishes kernel space and userspace: sysctl_oid / libsysctl_object, CTL_MAXNAME / LIBSYSCTL_IDMAXLEVEL, etc. Anyway can the prefix "libsysctl" be kept in libutil? Should I use a new prefix "sysctl_util_" or "util_sysctl_" for libutil? What do you prefer?

lib/libsysctl/libsysctl.3

293 ↗

(On Diff #52443)

fixed, change: "heap" -> "allocated memory"

lib/libsysctl/libsysctl.c

90 ↗

(On Diff #52443)

fixed, delete: Casting to void* before passing as a void* argument

171 ↗

(On Diff #52443)

fixed: done in a temporary, *idnextlen only updated on success.

226 ↗

(On Diff #52443)

fixed, delete: cast malloc()

241 ↗

(On Diff #52443)

fixed, delete: redundant to the bzero.

243 ↗

(On Diff #52443)

fixed, change: bzero() -> memset()

255 ↗

(On Diff #52443)

fixed, add: libsysctl_freeobject(obj) after each malloc failure

315 ↗

(On Diff #52443)

fixed: used 'uncrustify' to check style(9)

448 ↗

(On Diff #52443)

fixed: Declarations are at the top of the functions.

lib/libsysctl/libsysctl.h

44 ↗

(On Diff #52443)

fixed: added variable names in the function prototypes.

79 ↗

(On Diff #52443)

(Thanks you, very important suggestion).
Fixed, delete: namelen, desclen, labellen, fmtlen.
remain: idlen, id isn' t a static array then a can't use nitems() to get back the level of the entry.

90 ↗

(On Diff #52443)

Fixed, change: "childs" -> "children"
(I left the UK 3 years ago, it' s time to refresh my English).

[bapt]

In D18696#400437, @bapt wrote:

why not adding that to libutils instead of adding another library?

I used "libsysctl_" prefix to avoid naming conflict from <sys/sysctl.h>. Moreover a programmer easily distinguishes kernel space and userspace: sysctl_oid / libsysctl_object, CTL_MAXNAME / LIBSYSCTL_IDMAXLEVEL, etc. Anyway can the prefix "libsysctl" be kept in libutil? Should I use a new prefix "sysctl_util_" or "util_sysctl_" for libutil? What do you prefer?

I do not see why you could not, libutil is not a consistent library in term of API it is more a collection of "hey that is useful but we don't want to bloat our libc".

For example looks at pw_util(3) it is prefixed by pw_
I have no strong opinion about the prefix you should use, libsysctl sounds ok to me, don't know what others thinks

[asiciliano]

Complete manual, checked by "mandoc -Tlint" and "igor -Dd".

comments and suggestions applied,
(particularly: avoid memory leak, delete some struct member for alignment constraints and fixed style(9) ).

[dab]

lib/libsysctl/libsysctl.3

343–349 ↗

(On Diff #52741)

Again, here it both says the library isn't designed to get/set kernel state, yet the macros are defined anyway. See comments above about this.

If these are kept, I would rephrase the text here:

[...]
.Xr sysctl 3 .
However, some macros are defined for this purpose:
[...]

lib/libsysctl/libsysctl.h

152 ↗

(On Diff #52741)

These are very thin wrappers around sysctl() and I don't think provide significant value. Why define them?

At first I thought maybe the GET was going to address the common case where the value length is not known and you have to round-trip twice, first to get the length and then a second time to get the actual value, but that isn't the case. I think that would provide value.

I would just delete these as they are currently defined.

158 ↗

(On Diff #52741)

nit: s/macro is/macros are/

[asiciliano]

lib/libsysctl/libsysctl.3

343–349 ↗

(On Diff #52741)

answer up

lib/libsysctl/libsysctl.h

152 ↗

(On Diff #52741)

Thank you for your comment,
OK I 'll delete _GETVALUE(), _SETVALUE() and update the manual.

first to get the length and then a second time to get the actual value

I think you mean: LIBSYSCTL_VALUELEN(id, idlevel, size) sysctl(id, idlevel, NULL, size, NULL, 0) , if you wish I could add it, anyway it's right: these are very thin wrappers.

158 ↗

(On Diff #52741)

answer up

[asiciliano]

libsysctl moved to libutil,
thin wrappers deleted ( GETVALUE() and SETVALUE() ),
manual updated and
fixed style(9).

[danfe]

In D18696#401385, @alfix86_gmail.com wrote:

I used "libsysctl_" prefix to avoid naming conflict from <sys/sysctl.h>. Moreover a programmer easily distinguishes kernel space and userspace: sysctl_oid / libsysctl_object, CTL_MAXNAME / LIBSYSCTL_IDMAXLEVEL, etc.

I understand the motivation, that's a fair point.

Anyway can the prefix "libsysctl" be kept in libutil? Should I use a new prefix "sysctl_util_" or "util_sysctl_" for libutil? What do you prefer?

Personally I find lib prefix redundant for library functions in general, yet sysctl_util_ looks a bit too verbose. You might want to take a look at /usr/include/capsicum_helpers.h and their caph prefix (sysctlh or perhaps sysctl_tree since these functions mainly revolve around sysctl trees).

Style(9)-wise, I think you could trim excessive double newlines (between the functions and sometimes even within a function).

[asiciliano]

Thank you for your comment,

In D18696#402107, @danfe wrote:

In D18696#401385, @alfix86_gmail.com wrote:

I used "libsysctl_" prefix to avoid naming conflict from <sys/sysctl.h>. Moreover a programmer easily distinguishes kernel space and userspace: sysctl_oid / libsysctl_object, CTL_MAXNAME / LIBSYSCTL_IDMAXLEVEL, etc.

Personally I find lib prefix redundant for library functions in general, yet sysctl_util_ looks a bit too verbose. You might want to take a look at /usr/include/capsicum_helpers.h and their caph prefix (sysctlh or perhaps sysctl_tree since these functions mainly revolve around sysctl trees).

Then I could use sysctlmif_ and rename files to sysctlmibinfo.h/c/3 .

Style(9)-wise, I think you could trim excessive double newlines

I' ll update to fix.

[danfe]

In D18696#402995, @alfix86_gmail.com wrote:

Then I could use sysctlmif_ and rename files to sysctlmibinfo.h/c/3 .

That looks concise yet specific enough and makes sense, I think like it. :-)

[asiciliano]

• Change prefix: libsysctl_ -> sysctlmif_
• manual page: update API
• rename files: libsysctl.h/c/3 -> sysctlmibinfo.h/c/3
• update lib/libutil/Makefile
• fix style(9): delete 'double newlines'

[debdrup]

Found two things which may need some TLC.

lib/libutil/sysctlmibinfo.3

146

Maybe you should rephrase this as "as it is not designed to get and set entry values, anyone wishing to do this should see sysctl(3)"?

211

Doesn't this sentence become a double negative?

[dab]

lib/libutil/sysctlmibinfo.3

211

s/has not a/has the/

[asiciliano]

• add: __FBSDID("$FreeBSD$").
• indent: sysctlmif_internal_subtree().
• change: SYSCTLMIF_IDMAXLEVEL -> SYSCTLMIF_MAXIDLEVEL ( = CTL_MAXNAME).
• change: mib-size for magic nodes to mib[CTL_MAXNAME + 2] (like in sbin/sysctl.c).
• add free(object->fmt) in sysctlmif_freeobject() to avoid memory leak.
• manual page: fix comments and update API.

[asiciliano]

Thank you for comments, I updated the manual page

lib/libutil/sysctlmibinfo.3

146

Ok, I'll update

211

Thank you, I' ll change to fix

[rpokala]

Question: why "sysctlmif", as opposed to "sysctlmib"? Since this is about walking the MIB tree?

[asiciliano]

In D18696#406259, @rpokala wrote:

Question: why "sysctlmif", as opposed to "sysctlmib"? Since this is about walking the MIB tree?

Thank you for the comment,

"sysctlmif" - "sysctlmibinfo", <sysctlmibinfo.h>, man sysctlmibinfo, sysctlmif_*(), etc. , describes the purpose of this API: getting entry-info (name, desc, next, ...) and possibly building some list or (sub) tree of "mif_objects". Before I thought to use "sysctlmib" to rename "libsysctl" , but it seemed to me that it would be suitable for a "complete" mib API.

Anyway I have no strong opinion about the prefix/naming, What do you prefer? sysctlmib.h/.c/.3/_* ?

[rpokala]

Anyway I have no strong opinion about the prefix/naming, What do you prefer? sysctlmib.h/.c/.3/_* ?

No changes, I was just curious. :-) Your explanation makes sense. Thanks!

[asiciliano]

Abandon Revision: This library is obsoleted by <devel/libsysctlmibinfo2>,
Thank you for the suggestions and reviews.