This program prints the number of CPU threads it can run on, while respecting cpusets (or not, depending on switches). It aims to be compatible with nproc as found in GNU coreutils.
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Minor style improvements, nitpicking and questions.
bin/nproc/nproc.c | ||
---|---|---|
43 | This is an invitation for a discussion more than a legitimate suggestion, but why do we seem to prefer fprintf calls (and multiple of them) in base userland instead of a single fputs (or fputs_unlocked maybe) call? | |
68 | I can imagine you had a reason not to do it like this, but I can't think of anything specific. | |
95 | Wouldn't a break do the same? It'd look more consistent and idiomatic, in my opinion. If I'm not wrong about this, I think it's worth fixing to make the new program a bit more consistent from the start. | |
129 |
bin/nproc/nproc.c | ||
---|---|---|
95 | I missed that usage exits with 1. But still you could parameterize help and call it directly, and then remove usage. |
TODO: write the manpage
Consider taking this as a starting point: https://reviews.freebsd.org/P553
bin/nproc/nproc.c | ||
---|---|---|
68 |
You've explained this, but in the comment for the whole function, so I missed it. |
bin/nproc/nproc.c | ||
---|---|---|
43 | Premature optimization. The compiler will replace the fprintf() call with more efficient code if appropriate. |
bin/nproc/Makefile | ||
---|---|---|
2 | This is not needed. |
bin/nproc/Makefile | ||
---|---|---|
6 | I think you're missing the directive to install the manual page? |
bin/nproc/Makefile | ||
---|---|---|
6 | No, it's implicit, as long as it goes in expected section. |
bin/nproc/nproc.1 | ||
---|---|---|
45 | You'll need this to prevent mandoc from inserting random line break within the author section. |
bin/nproc/nproc.c | ||
---|---|---|
90 | Maybe take a page from lld e.g. something like but just a little bikeshed point |
bin/nproc/nproc.c | ||
---|---|---|
90 | that would miss the point, i noted in the ocmment this is whitespace-compatible with the original: nproc (GNU coreutils) 8.32 if some funny program decides to --version and grab the version, they will get a sane number |
bin/nproc/nproc.c | ||
---|---|---|
90 | but some silly program may equally do something like nproc --version | grep -q "GNU coreutils" |
bin/nproc/nproc.c | ||
---|---|---|
77 | other tools like ls don't do it, so i'm going to scratch this bit |
bin/nproc/nproc.c | ||
---|---|---|
90 | that would be most peculiar |
bin/nproc/nproc.c | ||
---|---|---|
77 | Just because older code gets it wrong doesn't mean we shouldn't get it right in new code. |
bin/nproc/nproc.c | ||
---|---|---|
90 | not really, the reason lld has the peculiar version string is that libtool does essentially this (I think it may just grep for GNU though) |
bin/nproc/nproc.c | ||
---|---|---|
90 | but the code cannot claim to be gnu coreutils and even then, grabbing the version number from it runs into the whitespace issue i mentioned. |
bin/nproc/nproc.c | ||
---|---|---|
131 |
bin/nproc/nproc.c | ||
---|---|---|
90–93 | The code didn't check whether the result of strtol would fit into the int ignore. This means a user could cause an overflow, which is UB. In reality this means that very large values might wrap around into the accepted range and the program would run successfully without doing what the user intended. |
bin/nproc/nproc.c | ||
---|---|---|
90–93 | fair point, thanks! |
bin/nproc/nproc.1 | ||
---|---|---|
2 | I doubt this has any use in manual pages. It is used in C sources in hope that an indent run will not reformat the comment. | |
49 | I wonder if this won't be confusing. FreeBSD 14 will be the first FreeBSD to have nproc in base, but it wasn't the first system to provide it (to us it's obvious, but we're not writing manual pages just for ourselves). |