Page MenuHomeFreeBSD

Move cpuset's parselist function into libutil
ClosedPublic

Authored by bapt on Oct 31 2017, 2:39 PM.
Tags
None
Referenced Files
F105831725: D12873.diff
Sat, Dec 21, 9:44 AM
Unknown Object (File)
Oct 4 2024, 9:23 PM
Unknown Object (File)
Oct 4 2024, 9:20 PM
Unknown Object (File)
Oct 4 2024, 9:12 PM
Unknown Object (File)
Oct 4 2024, 7:23 PM
Unknown Object (File)
Sep 28 2024, 1:28 PM
Unknown Object (File)
Sep 24 2024, 11:41 AM
Unknown Object (File)
Sep 24 2024, 11:41 AM

Details

Summary

Too allow to add cpuset(2) functionality to more utilities than just cpuset(1)
move the parselist code into libutil

While here, make the code a little more "library" friendly, by returning a range
of various errors so that the consumer can check for them and report appropriate
error message to the users

(One of the planed usage is the jail(8) utility)

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 12333
Build 12621: arc lint + arc unit

Event Timeline

Fixes for the man page.

lib/libutil/cpuset.3
53

You need to do a line break after a sentence stop.

60

s/succesfull/successful/

70

s/CPU/CPUs/

Note that all my comments on the cpuset_parselist() implementation also apply to the original version in cpuset.c; therefore, they could be fixed in a separate commit.

A unit test would be nice.

lib/libutil/cpuset.3
41

The actual implementation has one asterisk with mask, which matches the below description.

59

The Er macro seems meant for errno values only. Perhaps use Dv instead of Bq Er.

70

This should mention CPU_SETSIZE; otherwise someone may think it is about the number of CPUs in the system.

lib/libutil/cpuset.c
39

should use const char *list (which also needs adding some const below)

43

Perhaps use an unsigned type to avoid undefined behaviour on overflow (see below).

55

It is annoying but the way to pass a char to isdigit and similar macros is to cast it to unsigned char first.

56–60

atoi causes undefined behaviour on overflow; consider strtoul or an open-coded loop.

lib/libutil/libutil.h
205

I don't like depending on the include order like this but it is convention in this file.

Fix manpage issue
Change prototype to using const.
Other changes deserves a dedicated commit that I will do later

Remove files that crept in

bapt marked 7 inline comments as done.Nov 11 2017, 3:08 PM

see D13046 for the tests

lib/libutil/cpuset.c
43

This is will be fixed in a another commit

55

in another commit

56–60

in a separated commit

lib/libutil/cpuset.c
47–52

This behavior should really be split off separately from the list handling. I would like to be able to use the list handling code for domains, which also won't be cpumask_t. Instead I would pass in the bitset type and size that cpumask is derived from. So eliminating cpu specific code from this function will make it more useful and generic in general. Other code man choose to derive bit manipulation functions from the bitset macros in the future and this would be compatible.

Accepted with a minor change to the man page.

lib/libutil/cpuset.3
73–74

This sentence is incomplete. It could be spliced to the previous sentence with a comma, or formulated differently.

lib/libutil/cpuset.c
47–52

I think the cpuset_parselist() API is useful as is; the more general ranges-of-numbers code can be extracted out in another commit.

This revision is now accepted and ready to land.Nov 22 2017, 11:01 PM
wblock added inline comments.
lib/libutil/cpuset.3
73–74
The maximum number of CPUs is
.Va CPU_SETSIZE .