* Re: [PATCH v2] lscpu: fix incorrect number of sockets during hotplug
From: Karel Zak @ 2024-11-04 12:08 UTC (permalink / raw)
To: Anjali K; +Cc: util-linux, anushree.mathur
In-Reply-To: <20241104063226.172077-1-anjalik@linux.ibm.com>
Thank you, it looks good. I made some cosmetic changes to the patch
and created a pull request to run it through CI tests on GitHub.
https://github.com/util-linux/util-linux/pull/3265
Thanks!
On Mon, Nov 04, 2024 at 12:02:26PM GMT, Anjali K wrote:
> sys-utils/lscpu-topology.c | 71 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 64 insertions(+), 7 deletions(-)
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [PATCH] lscpu: fix incorrect number of sockets during hotplug
From: Anjali K @ 2024-11-04 6:37 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux, anushree.mathur, anjalik
In-Reply-To: <psbp7ofmppegfcgz6w4jzaqx5ek5yeoc7zqeoh7k5bmdpp4ivg@4xkmw5pmsp4t>
On 30/10/24 16:25, Karel Zak wrote:
> Would be better to allocate this only once in cputype_read_topology()
> and reuse it for all the arrays and CPUs?
Thank you for your review. I have made the change and sent v2 of the patch. Please review.
Thank you,
Anjali K
^ permalink raw reply
* [PATCH v2] lscpu: fix incorrect number of sockets during hotplug
From: Anjali K @ 2024-11-04 6:32 UTC (permalink / raw)
To: util-linux; +Cc: kzak, anushree.mathur, anjalik
lscpu sometimes shows incorrect 'Socket(s)' value if a hotplug operation
is running.
On a 32 CPU 2-socket system, the expected output is as shown below:
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 32
On-line CPU(s) list: 0-31
Model name: POWER10 (architected), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 2
Socket(s): 2
On the same system, if hotplug is running along with lscpu, it shows
"Socket(s):" as 3 and 4 incorrectly sometimes.
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 32
On-line CPU(s) list: 0-11,16-31
Off-line CPU(s) list: 12-15
Model name: POWER10 (architected), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 1
Socket(s): 3
The number of sockets is considered as the number of unique core_siblings
CPU groups. The issues causing the number of sockets to sometimes be
higher during hotplug is:
1. The core_siblings of CPUs on the same socket are different because a CPU
on the socket has been onlined/offlined in between. In the below example,
nr sockets was wrongly incremented for CPU 5 though CPU 4 and 5 are on the
same socket because their core_siblings was different as CPU 12 was onlined
in between.
CPU: 4
core_siblings: ff f0 0 0 0 0 0 0
CPU: 5
core_siblings: ff f8 0 0 0 0 0 0
2. The core_siblings file of a CPU is created when a CPU is onlined. It may
have an invalid value for some time until the online operation is fully
complete. In the below example, nr sockets is wrongly incremented because
the core_siblings of CPU 14 was 0 as it had just been onlined.
CPU: 14
core_siblings: 0 0 0 0 0 0 0 0
To fix this, make the below changes:
1. Instead of considering CPUs to be on different sockets if their
core_siblings masks are unequal, consider them to be on different sockets
only if their core_siblings masks don't have even one common CPU. Then CPUs
on the same socket will be correctly identified even if offline/online
operations happen while they are read if at least one CPU in the socket is
online during both reads.
2. Check if a CPU's hotplug operation has been completed before using its
core_siblings file
Reported-by: Anushree Mathur <anushree.mathur@linux.vnet.ibm.com>
Signed-off-by: Anjali K <anjalik@linux.ibm.com>
---
sys-utils/lscpu-topology.c | 71 ++++++++++++++++++++++++++++++++++----
1 file changed, 64 insertions(+), 7 deletions(-)
diff --git a/sys-utils/lscpu-topology.c b/sys-utils/lscpu-topology.c
index e3742e319..7fb4a1af9 100644
--- a/sys-utils/lscpu-topology.c
+++ b/sys-utils/lscpu-topology.c
@@ -17,19 +17,25 @@
#include <unistd.h>
#include <string.h>
#include <stdio.h>
+#include <ctype.h>
#include "lscpu.h"
/* add @set to the @ary, unnecessary set is deallocated. */
-static int add_cpuset_to_array(cpu_set_t **ary, size_t *items, cpu_set_t *set, size_t setsize)
+static int add_cpuset_to_array(cpu_set_t **ary, size_t *items, cpu_set_t *set, size_t setsize, cpu_set_t *common_cpus_set)
{
size_t i;
if (!ary)
return -EINVAL;
+ /*
+ * Check if @set has no cpu in common with the cpusets
+ * saved in @ary and if so append @set to @ary.
+ */
for (i = 0; i < *items; i++) {
- if (CPU_EQUAL_S(setsize, set, ary[i]))
+ CPU_AND_S(setsize, common_cpus_set, set, ary[i]);
+ if (CPU_COUNT_S(setsize, common_cpus_set))
break;
}
if (i == *items) {
@@ -98,14 +104,49 @@ void lscpu_sort_caches(struct lscpu_cache *caches, size_t n)
qsort(caches, n, sizeof(struct lscpu_cache), cmp_cache);
}
+/*
+ * Get the hotplug state number representing a completely online
+ * cpu from /sys/devices/system/cpu/hotplug/state
+ */
+static int get_online_state(struct path_cxt *sys)
+{
+ int hp_online_state_val, page_size, rc;
+ char *buf, *strp;
+
+ hp_online_state_val = -1;
+
+ /* sysfs text files have size = page size */
+ page_size = getpagesize();
+
+ buf = (char *)malloc(page_size);
+ if (!buf)
+ goto done;
+ rc = ul_path_readf_buffer(sys, buf, page_size, "hotplug/states");
+ if (rc <= 0)
+ goto done;
+
+ strp = strstr(buf, ": online");
+ if (!strp)
+ goto done;
+
+ strp--; /* get digits before ': online' */
+ while (strp >= buf && isdigit(*strp))
+ strp--;
+ hp_online_state_val = atoi(strp + 1);
+
+done:
+ free(buf);
+ return hp_online_state_val;
+}
/* Read topology for specified type */
static int cputype_read_topology(struct lscpu_cxt *cxt, struct lscpu_cputype *ct)
{
size_t i, npos;
struct path_cxt *sys;
- int nthreads = 0, sw_topo = 0;
+ int nthreads = 0, sw_topo = 0, rc, hp_state, hp_online_state;
FILE *fd;
+ cpu_set_t *temp_set;
sys = cxt->syscpu; /* /sys/devices/system/cpu/ */
npos = cxt->npossibles; /* possible CPUs */
@@ -113,6 +154,12 @@ static int cputype_read_topology(struct lscpu_cxt *cxt, struct lscpu_cputype *ct
DBG(TYPE, ul_debugobj(ct, "reading %s/%s/%s topology",
ct->vendor ?: "", ct->model ?: "", ct->modelname ?:""));
+ hp_online_state = get_online_state(sys);
+
+ temp_set = CPU_ALLOC(cxt->maxcpus);
+ if (!temp_set)
+ return -EINVAL;
+
for (i = 0; i < cxt->npossibles; i++) {
struct lscpu_cpu *cpu = cxt->cpus[i];
cpu_set_t *thread_siblings = NULL, *core_siblings = NULL;
@@ -127,6 +174,15 @@ static int cputype_read_topology(struct lscpu_cxt *cxt, struct lscpu_cputype *ct
"cpu%d/topology/thread_siblings", num) != 0)
continue;
+ /*
+ * Ignore cpus which are not fully online.
+ * If hp_online_state is negative/zero or rc is negative,
+ * online state could not be read correctly, skip this check.
+ */
+ rc = ul_path_readf_s32(sys, &hp_state, "cpu%d/hotplug/state", num);
+ if (hp_online_state > 0 && rc >= 0 && hp_state != hp_online_state)
+ continue;
+
/* read topology maps */
ul_path_readf_cpuset(sys, &thread_siblings, cxt->maxcpus,
"cpu%d/topology/thread_siblings", num);
@@ -163,15 +219,16 @@ static int cputype_read_topology(struct lscpu_cxt *cxt, struct lscpu_cputype *ct
/* add to topology maps */
if (thread_siblings)
- add_cpuset_to_array(ct->coremaps, &ct->ncores, thread_siblings, cxt->setsize);
+ add_cpuset_to_array(ct->coremaps, &ct->ncores, thread_siblings, cxt->setsize, temp_set);
if (core_siblings)
- add_cpuset_to_array(ct->socketmaps, &ct->nsockets, core_siblings, cxt->setsize);
+ add_cpuset_to_array(ct->socketmaps, &ct->nsockets, core_siblings, cxt->setsize, temp_set);
if (book_siblings)
- add_cpuset_to_array(ct->bookmaps, &ct->nbooks, book_siblings, cxt->setsize);
+ add_cpuset_to_array(ct->bookmaps, &ct->nbooks, book_siblings, cxt->setsize, temp_set);
if (drawer_siblings)
- add_cpuset_to_array(ct->drawermaps, &ct->ndrawers, drawer_siblings, cxt->setsize);
+ add_cpuset_to_array(ct->drawermaps, &ct->ndrawers, drawer_siblings, cxt->setsize, temp_set);
}
+ CPU_FREE(temp_set);
/* s390 detects its cpu topology via /proc/sysinfo, if present.
* Using simply the cpu topology masks in sysfs will not give
base-commit: 9e2aafe5a493284615a17572c4ead4737bcc66a3
--
2.46.0
^ permalink raw reply related
* Re: [PATCH 2/4] hardlink: add --list-duplicates and --zero
From: наб @ 2024-10-31 14:56 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
In-Reply-To: <ivgbtqt2qjlpzxgh6tkdapfohjwu677heddtn7cpkpaqjyoi6g@brl4uxsj6lsv>
[-- Attachment #1: Type: text/plain, Size: 3695 bytes --]
On Thu, Oct 31, 2024 at 09:58:19AM +0100, Karel Zak wrote:
> On Mon, Oct 28, 2024 at 07:19:30PM GMT, наб wrote:
> > --list-duplicates codifies what everyone keeps re-implementing with
> > find -exec b2sum or src:perforate's finddup or whatever.
> >
> > hardlink already knows this, so make the data available thusly,
> > in a format well-suited for pipeline processing
> > (fixed-width key for uniq/cut/&c.,
> > tab delimiter for cut &a.,
> > -z for correct filename handling).
>
> Why do we need a 16-byte discriminator? The list consists of absolute
> paths, so it should be unique enough. This seems like an unusual
> thing,
Well, the point is to have a list of lists of files, right.
hardlink(1) finds, within the given domain,
a set of sets of "these files are identical"
(or, the logical set of "these are the link names of this file"
for all eligible files).
The only way to flatten this is to a single-layer list is by having a
list of filenames discriminated by the set in which they belong, so
[[a, b], [c, d, e]]
discriminated as
0 a
0 b
1 c
1 d
1 e
which allows you to reconstuct the sets live while stream-processing
(the implementation uses a unique ASLR-randomised discriminator
because the order isn't stable anyway I think? but same difference).
A list of just filenames is useless.
> as I cannot recall any other tool that uses something like
> this.
This is what the b2sum/sha1sum/&c. family does.
(And, in a worse and less structured manner, sum/cksum.)
If you were to implement this with one of those,
you'd do something like
find -type f -exec b2sum {} + | sort | uniq -Dw128
which works but has other issues
(not tab-delimited, slow, harder than necessary to configure,
actually you want to sprinkle -z everywhere, &c.).
There's no other commonly-accepted program that does this,
I want to say it's because (a) hardlink is The Util-Linux Implementation
which doesn't necessarily exclude others, but certainly discourages them,
(b) hardlink doesn't tell you, so (c) if you're querying something
in a way that hardlink doesn't support,
you're doing it ad-hoc with whatever you think of,
and you're wondering why hardlink won't just tell you.
Debian has, in src:perforate, finddup, which implements this.
It's very much 1996 (it reads the whole file into memory, in Perl,
before uniquifying by MD5(!)), and the output format is
84 './build-output/dsh-0.25.10.obsolete.1730308699.8166876/debian/watch' './build-output/dsh-0.25.10.obsolete.1730308753.583969/debian/watch'
84 './build-output/dsh-0.25.10.obsolete.1730306971.4697168/debian/watch' './build-output/dsh-0.25.10.obsolete.1730306296.9378986/debian/watch' './build-output/dsh-0.25.10.obsolete.1730306808.9797611/debian/watch'
which is not in any way useful (the prefix is the size).
This then lets you process the equivalence sets separately
(I hope to replace this monstrosity I run commonly:
find -exec b2sum {} + | sort | mawk '{h = substr($0, 1, 128); fn = substr($0, 1 + 128 + 2); if(h == hash) {tgt = "." fname; split(fn, curs, "/"); if(curs[2] == fnames[2]) tgt = fnames[3]; print "[ -s \"" fn "\" ] && ln -sf -- \"" tgt "\" \"" fn "\""} else {hash = h; fname = fn; split(fname, fnames, "/")}}' | sh
with something hardlink--l-based.
Actually this would want hardlink -L ideally;
would you accept a patch that adds -L?).
On Thu, Oct 31, 2024 at 09:51:00AM +0100, Karel Zak wrote:
> The new option should also be added to the "bash-completion/hardlink"
> file. However, I can fix this after merging locally.
I missed this. I'll include it in v2 if we get to v2 but if we don't,
please do, thanks.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] hardlink: add --list-duplicates and --zero
From: Karel Zak @ 2024-10-31 8:58 UTC (permalink / raw)
To: наб; +Cc: util-linux
In-Reply-To: <b22071e3546940d6d0fa6e1d9d03f292e18229e2.1730139540.git.nabijaczleweli@nabijaczleweli.xyz>
On Mon, Oct 28, 2024 at 07:19:30PM GMT, наб wrote:
> --list-duplicates codifies what everyone keeps re-implementing with
> find -exec b2sum or src:perforate's finddup or whatever.
>
> hardlink already knows this, so make the data available thusly,
> in a format well-suited for pipeline processing
> (fixed-width key for uniq/cut/&c.,
> tab delimiter for cut &a.,
> -z for correct filename handling).
Why do we need a 16-byte discriminator? The list consists of absolute
paths, so it should be unique enough. This seems like an unusual
thing, as I cannot recall any other tool that uses something like
this.
> +*-l*, *--list-duplicates*::
> +Don't link anything, but list the absolute path of every duplicate file, one per line, preceded by a unique 16-byte discriminator and a tab.
...
> + printf("%016zu\t%s%c", (size_t)other, l->path, opts.line_delim);
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [PATCH 2/4] hardlink: add --list-duplicates and --zero
From: Karel Zak @ 2024-10-31 8:51 UTC (permalink / raw)
To: наб; +Cc: util-linux
In-Reply-To: <b22071e3546940d6d0fa6e1d9d03f292e18229e2.1730139540.git.nabijaczleweli@nabijaczleweli.xyz>
On Mon, Oct 28, 2024 at 07:19:30PM GMT, наб wrote:
> --list-duplicates codifies what everyone keeps re-implementing with
> find -exec b2sum or src:perforate's finddup or whatever.
>
> hardlink already knows this, so make the data available thusly,
> in a format well-suited for pipeline processing
> (fixed-width key for uniq/cut/&c.,
> tab delimiter for cut &a.,
> -z for correct filename handling).
> ---
> misc-utils/hardlink.1.adoc | 6 ++++++
> misc-utils/hardlink.c | 21 ++++++++++++++++++++-
> 2 files changed, 26 insertions(+), 1 deletion(-)
The new option should also be added to the "bash-completion/hardlink"
file. However, I can fix this after merging locally.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [PATCH 1/4] hardlink.1: directory|file is mandatory
From: Karel Zak @ 2024-10-31 8:48 UTC (permalink / raw)
To: наб; +Cc: util-linux
In-Reply-To: <5acde6a911f086ab8d2314c5b76eb76075140941.1730139540.git.nabijaczleweli@nabijaczleweli.xyz>
Hi,
I have created a pull request to test the patches on our CI.
https://github.com/util-linux/util-linux/pull/3260https://github.com/util-linux/util-linux/pull/3260
Thanks,
Karel
On Mon, Oct 28, 2024 at 07:19:14PM GMT, наб wrote:
> ---
> misc-utils/hardlink.1.adoc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/misc-utils/hardlink.1.adoc b/misc-utils/hardlink.1.adoc
> index d11045941..b6f07ba70 100644
> --- a/misc-utils/hardlink.1.adoc
> +++ b/misc-utils/hardlink.1.adoc
> @@ -18,7 +18,7 @@
>
> == SYNOPSIS
>
> -*hardlink* [options] [_directory_|_file_]...
> +*hardlink* [options] _directory_|_file_...
>
> == DESCRIPTION
>
> --
> 2.39.2
>
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [PATCH] lscpu: fix incorrect number of sockets during hotplug
From: Karel Zak @ 2024-10-30 10:55 UTC (permalink / raw)
To: Anjali K; +Cc: util-linux, anushree.mathur
In-Reply-To: <20241018104335.3481856-1-anjalik@linux.ibm.com>
HI,
sorry for delay with review.
On Fri, Oct 18, 2024 at 04:13:35PM GMT, Anjali K wrote:
> /* add @set to the @ary, unnecessary set is deallocated. */
> -static int add_cpuset_to_array(cpu_set_t **ary, size_t *items, cpu_set_t *set, size_t setsize)
> +static int add_cpuset_to_array(cpu_set_t **ary, size_t *items, cpu_set_t *set, size_t setsize, int maxcpus)
> {
> + cpu_set_t *common_cpus_set;
> size_t i;
>
> if (!ary)
> return -EINVAL;
>
> + common_cpus_set = CPU_ALLOC(maxcpus);
> + if (!common_cpus_set)
> + return -EINVAL;
Would be better to allocate this only once in cputype_read_topology()
and reuse it for all the arrays and CPUs?
The rest looks good. Thanks!
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* [PATCH 1/4] hardlink.1: directory|file is mandatory
From: наб @ 2024-10-28 18:19 UTC (permalink / raw)
To: util-linux
[-- Attachment #1: Type: text/plain, Size: 433 bytes --]
---
misc-utils/hardlink.1.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/misc-utils/hardlink.1.adoc b/misc-utils/hardlink.1.adoc
index d11045941..b6f07ba70 100644
--- a/misc-utils/hardlink.1.adoc
+++ b/misc-utils/hardlink.1.adoc
@@ -18,7 +18,7 @@
== SYNOPSIS
-*hardlink* [options] [_directory_|_file_]...
+*hardlink* [options] _directory_|_file_...
== DESCRIPTION
--
2.39.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 4/4] hardlink: re-raise SIGINT instead of exiting
From: наб @ 2024-10-28 18:19 UTC (permalink / raw)
To: util-linux
In-Reply-To: <5acde6a911f086ab8d2314c5b76eb76075140941.1730139540.git.nabijaczleweli@nabijaczleweli.xyz>
[-- Attachment #1: Type: text/plain, Size: 3036 bytes --]
Every TRUE return was either exit(1) or "return error;" anyway,
this is more correct (exit-status-wise) and doesn't hang pipelines weirdly
This also removes a seemingly-extraneous newline written to the standard
output stream in the interrupt handler itself?
---
misc-utils/hardlink.c | 33 ++++++++++-----------------------
1 file changed, 10 insertions(+), 23 deletions(-)
diff --git a/misc-utils/hardlink.c b/misc-utils/hardlink.c
index 8c5f24aad..e996f53b5 100644
--- a/misc-utils/hardlink.c
+++ b/misc-utils/hardlink.c
@@ -428,23 +428,20 @@ static void print_stats(void)
/**
* handle_interrupt - Handle a signal
- *
- * Returns: %TRUE on SIGINT, SIGTERM; %FALSE on all other signals.
*/
-static int handle_interrupt(void)
+static void handle_interrupt(void)
{
switch (last_signal) {
- case SIGINT:
- case SIGTERM:
- return TRUE;
case SIGUSR1:
print_stats();
putchar('\n');
break;
+ default:
+ signal(last_signal, SIG_DFL);
+ raise(last_signal);
+ break;
}
-
last_signal = 0;
- return FALSE;
}
#ifdef USE_XATTR
@@ -589,8 +586,7 @@ static int file_xattrs_equal(const struct file *a, const struct file *b)
// We now have two sorted tables of xattr names.
for (i = 0; i < n_a; i++) {
- if (handle_interrupt())
- goto exit; // user wants to quit
+ handle_interrupt();
if (strcmp(name_ptrs_a[i], name_ptrs_b[i]) != 0)
goto exit; // names at same slot differ
@@ -845,8 +841,7 @@ static int inserter(const char *fpath, const struct stat *sb,
int included;
int excluded;
- if (handle_interrupt())
- return 1;
+ handle_interrupt();
if (typeflag == FTW_DNR || typeflag == FTW_NS)
warn(_("cannot read %s"), fpath);
if (typeflag != FTW_F || !S_ISREG(sb->st_mode))
@@ -1072,8 +1067,7 @@ static void visitor(const void *nodep, const VISIT which, const int depth)
size_t nnodes, memsiz;
int may_reflink = 0;
- if (handle_interrupt())
- exit(EXIT_FAILURE);
+ handle_interrupt();
if (master->links == NULL)
continue;
@@ -1098,8 +1092,7 @@ static void visitor(const void *nodep, const VISIT which, const int depth)
for (other = master->next; other != NULL; other = other->next) {
int eq;
- if (handle_interrupt())
- exit(EXIT_FAILURE);
+ handle_interrupt();
assert(other != other->next);
assert(other->st.st_size == master->st.st_size);
@@ -1413,12 +1406,7 @@ static void to_be_called_atexit(void)
*/
static void sighandler(int i)
{
- UL_PROTECT_ERRNO;
- if (last_signal != SIGINT)
- last_signal = i;
- if (i == SIGINT)
- /* can't use stdio on signal handler */
- ignore_result(write(STDOUT_FILENO, "\n", sizeof("\n")-1));
+ last_signal = i;
}
int main(int argc, char *argv[])
@@ -1430,7 +1418,6 @@ int main(int argc, char *argv[])
sa.sa_flags = SA_RESTART;
sigfillset(&sa.sa_mask);
- /* If we receive a SIGINT, end the processing */
sigaction(SIGINT, &sa, NULL);
sigaction(SIGUSR1, &sa, NULL);
--
2.39.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 3/4] hardlink: fix 0-sized file processing
From: наб @ 2024-10-28 18:19 UTC (permalink / raw)
To: util-linux
In-Reply-To: <5acde6a911f086ab8d2314c5b76eb76075140941.1730139540.git.nabijaczleweli@nabijaczleweli.xyz>
[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]
The manual says that -s0 will process 0-sized files normally,
but as it stands (a) hardlink considers 0-sized files unlinkable
(so, with -l, unlistable) and (b) fileeq considers reading an empty
prologue to be an error
---
lib/fileeq.c | 2 +-
misc-utils/hardlink.c | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/fileeq.c b/lib/fileeq.c
index b40eba2b0..1f7f90ccb 100644
--- a/lib/fileeq.c
+++ b/lib/fileeq.c
@@ -465,7 +465,7 @@ static ssize_t get_intro(struct ul_fileeq *eq, struct ul_fileeq_data *data,
return -1;
rsz = read_all(fd, (char *) data->intro, sizeof(data->intro));
DBG(DATA, ul_debugobj(data, " read %zu bytes intro", sizeof(data->intro)));
- if (rsz <= 0)
+ if (rsz < 0)
return -1;
data->nblocks = 1;
}
diff --git a/misc-utils/hardlink.c b/misc-utils/hardlink.c
index 643df7cf2..8c5f24aad 100644
--- a/misc-utils/hardlink.c
+++ b/misc-utils/hardlink.c
@@ -649,8 +649,7 @@ static int file_xattrs_equal(const struct file *a, const struct file *b)
*/
static int file_may_link_to(const struct file *a, const struct file *b)
{
- return (a->st.st_size != 0 &&
- a->st.st_size == b->st.st_size &&
+ return (a->st.st_size == b->st.st_size &&
a->links != NULL && b->links != NULL &&
a->st.st_dev == b->st.st_dev &&
a->st.st_ino != b->st.st_ino &&
--
2.39.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 2/4] hardlink: add --list-duplicates and --zero
From: наб @ 2024-10-28 18:19 UTC (permalink / raw)
To: util-linux
In-Reply-To: <5acde6a911f086ab8d2314c5b76eb76075140941.1730139540.git.nabijaczleweli@nabijaczleweli.xyz>
[-- Attachment #1: Type: text/plain, Size: 4172 bytes --]
--list-duplicates codifies what everyone keeps re-implementing with
find -exec b2sum or src:perforate's finddup or whatever.
hardlink already knows this, so make the data available thusly,
in a format well-suited for pipeline processing
(fixed-width key for uniq/cut/&c.,
tab delimiter for cut &a.,
-z for correct filename handling).
---
misc-utils/hardlink.1.adoc | 6 ++++++
misc-utils/hardlink.c | 21 ++++++++++++++++++++-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/misc-utils/hardlink.1.adoc b/misc-utils/hardlink.1.adoc
index b6f07ba70..b85ba09d4 100644
--- a/misc-utils/hardlink.1.adoc
+++ b/misc-utils/hardlink.1.adoc
@@ -76,6 +76,12 @@
*-n*, *--dry-run*::
Do not act, just print what would happen.
+*-l*, *--list-duplicates*::
+Don't link anything, but list the absolute path of every duplicate file, one per line, preceded by a unique 16-byte discriminator and a tab.
+
+*-z*, *--zero*::
+Separate lines with a NUL instead of a newline in *-l* mode.
+
*-o*, *--ignore-owner*::
Link and compare files even if their owner information (user and group) differs. Results may be unpredictable.
diff --git a/misc-utils/hardlink.c b/misc-utils/hardlink.c
index e4b44c6c6..643df7cf2 100644
--- a/misc-utils/hardlink.c
+++ b/misc-utils/hardlink.c
@@ -189,6 +189,8 @@ static struct options {
unsigned int keep_oldest:1;
unsigned int prio_trees:1;
unsigned int dry_run:1;
+ unsigned int list_duplicates:1;
+ char line_delim;
uintmax_t min_size;
uintmax_t max_size;
size_t io_size;
@@ -206,6 +208,7 @@ static struct options {
.respect_xattrs = FALSE,
.keep_oldest = FALSE,
.prio_trees = FALSE,
+ .line_delim = '\n',
.min_size = 1,
.cache_size = 10*1024*1024
};
@@ -1152,6 +1155,10 @@ static void visitor(const void *nodep, const VISIT which, const int depth)
/* final cleanup */
for (other = begin; other != NULL; other = other->next) {
+ if (opts.list_duplicates && other->st.st_nlink > 1)
+ for (struct link *l = other->links; l; l = l->next)
+ printf("%016zu\t%s%c", (size_t)other, l->path, opts.line_delim);
+
if (ul_fileeq_data_associated(&other->data))
ul_fileeq_data_deinit(&other->data);
}
@@ -1182,6 +1189,8 @@ static void __attribute__((__noreturn__)) usage(void)
" lowest hardlink count\n"), out);
fputs(_(" -M, --minimize reverse the meaning of -m\n"), out);
fputs(_(" -n, --dry-run don't actually link anything\n"), out);
+ fputs(_(" -l, --list-duplicates print every group of duplicate files\n"), out);
+ fputs(_(" -z, --zero delimit output with NULs instead of newlines\n"), out);
fputs(_(" -o, --ignore-owner ignore owner changes\n"), out);
fputs(_(" -F, --prioritize-trees files found in the earliest specified top-level\n"
" directory have higher priority (lower precedence\n"
@@ -1223,7 +1232,7 @@ static int parse_options(int argc, char *argv[])
OPT_REFLINK = CHAR_MAX + 1,
OPT_SKIP_RELINKS
};
- static const char optstr[] = "VhvndfpotXcmMFOx:y:i:r:S:s:b:q";
+ static const char optstr[] = "VhvndfpotXcmMFOlzx:y:i:r:S:s:b:q";
static const struct option long_options[] = {
{"version", no_argument, NULL, 'V'},
{"help", no_argument, NULL, 'h'},
@@ -1252,6 +1261,8 @@ static int parse_options(int argc, char *argv[])
{"content", no_argument, NULL, 'c'},
{"quiet", no_argument, NULL, 'q'},
{"cache-size", required_argument, NULL, 'r'},
+ {"list-duplicates", no_argument, NULL, 'l'},
+ {"zero", no_argument, NULL, 'z'},
{NULL, 0, NULL, 0}
};
static const ul_excl_t excl[] = {
@@ -1329,6 +1340,14 @@ static int parse_options(int argc, char *argv[])
case 'b':
opts.io_size = strtosize_or_err(optarg, _("failed to parse I/O size"));
break;
+ case 'l':
+ opts.list_duplicates = TRUE;
+ opts.dry_run = TRUE;
+ quiet = TRUE;
+ break;
+ case 'z':
+ opts.line_delim = '\0';
+ break;
#ifdef USE_REFLINK
case OPT_REFLINK:
reflink_mode = REFLINK_AUTO;
--
2.39.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH] lscpu: fix incorrect number of sockets during hotplug
From: Anushree Mathur @ 2024-10-20 10:30 UTC (permalink / raw)
To: util-linux; +Cc: Anjali K, Anushree Mathur
In-Reply-To: <20241018104335.3481856-1-anjalik@linux.ibm.com>
Hi,
I have verified the patch and it works fine!Here is my analysis:
LSCPU O/P on my system without enabling and disabling cpus
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 384
On-line CPU(s) list: 0-383
Model name: POWER10 (raw), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 12
Socket(s): 4
Physical sockets: 2
Physical chips: 2
Physical cores/chip: 12
Before applying the patch
While repeatedly onlining/offlining cpus on my system , saw that socket
number is higher. It doesn't happen if i do online/offline of cpu single
time.
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 384
On-line CPU(s) list: 0-3,5-7,9,11-15,17-383
Off-line CPU(s) list: 4,8,10,16
Model name: POWER10 (raw), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 9
Socket(s): 5
Physical sockets: 2
Physical chips: 2
Physical cores/chip: 12
After applying the patch :
Tried the same scenario again to online and offline the cpus in a loop
continuously and it didn't show any wrong topology:
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 384
On-line CPU(s) list: 0-3,5-7,9,11-15,17-383
Off-line CPU(s) list: 4,8,10,16
Model name: POWER10 (raw), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 12
Socket(s): 4
Physical sockets: 2
Physical chips: 2
Physical cores/chip: 12
Tested-by: Anushree Mathur <anushree.mathur@linux.vnet.ibm.com>
Thanks,
Anushree Mathur
^ permalink raw reply
* [PATCH] lscpu: fix incorrect number of sockets during hotplug
From: Anjali K @ 2024-10-18 10:43 UTC (permalink / raw)
To: util-linux; +Cc: anushree.mathur, anjalik
lscpu sometimes shows incorrect 'Socket(s)' value if a hotplug operation
is running.
On a 32 CPU 2-socket system, the expected output is as shown below:
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 32
On-line CPU(s) list: 0-31
Model name: POWER10 (architected), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 2
Socket(s): 2
On the same system, if hotplug is running along with lscpu, it shows
"Socket(s):" as 3 and 4 incorrectly sometimes.
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 32
On-line CPU(s) list: 0-11,16-31
Off-line CPU(s) list: 12-15
Model name: POWER10 (architected), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 1
Socket(s): 3
The number of sockets is considered as the number of unique core_siblings
CPU groups. The issues causing the number of sockets to sometimes be
higher during hotplug is:
1. The core_siblings of CPUs on the same socket are different because a CPU
on the socket has been onlined/offlined in between. In the below example,
nr sockets was wrongly incremented for CPU 5 though CPU 4 and 5 are on the
same socket because their core_siblings was different as CPU 12 was onlined
in between.
CPU: 4
core_siblings: ff f0 0 0 0 0 0 0
CPU: 5
core_siblings: ff f8 0 0 0 0 0 0
2. The core_siblings file of a CPU is created when a CPU is onlined. It may
have an invalid value for some time until the online operation is fully
complete. In the below example, nr sockets is wrongly incremented because
the core_siblings of CPU 14 was 0 as it had just been onlined.
CPU: 14
core_siblings: 0 0 0 0 0 0 0 0
To fix this, make the below changes:
1. Instead of considering CPUs to be on different sockets if their
core_siblings masks are unequal, consider them to be on different sockets
only if their core_siblings masks don't have even one common CPU. Then CPUs
on the same socket will be correctly identified even if offline/online
operations happen while they are read if at least one CPU in the socket is
online during both reads.
2. Check if a CPU's hotplug operation has been completed before using its
core_siblings file
Reported-by: Anushree Mathur <anushree.mathur@linux.vnet.ibm.com>
Signed-off-by: Anjali K <anjalik@linux.ibm.com>
---
sys-utils/lscpu-topology.c | 70 ++++++++++++++++++++++++++++++++++----
1 file changed, 63 insertions(+), 7 deletions(-)
diff --git a/sys-utils/lscpu-topology.c b/sys-utils/lscpu-topology.c
index e3742e319..a24d93c03 100644
--- a/sys-utils/lscpu-topology.c
+++ b/sys-utils/lscpu-topology.c
@@ -17,21 +17,33 @@
#include <unistd.h>
#include <string.h>
#include <stdio.h>
+#include <ctype.h>
#include "lscpu.h"
/* add @set to the @ary, unnecessary set is deallocated. */
-static int add_cpuset_to_array(cpu_set_t **ary, size_t *items, cpu_set_t *set, size_t setsize)
+static int add_cpuset_to_array(cpu_set_t **ary, size_t *items, cpu_set_t *set, size_t setsize, int maxcpus)
{
+ cpu_set_t *common_cpus_set;
size_t i;
if (!ary)
return -EINVAL;
+ common_cpus_set = CPU_ALLOC(maxcpus);
+ if (!common_cpus_set)
+ return -EINVAL;
+
+ /*
+ * Check if @set has no cpu in common with the cpusets
+ * saved in @ary and if so append @set to @ary.
+ */
for (i = 0; i < *items; i++) {
- if (CPU_EQUAL_S(setsize, set, ary[i]))
+ CPU_AND_S(setsize, common_cpus_set, set, ary[i]);
+ if (CPU_COUNT_S(setsize, common_cpus_set))
break;
}
+ CPU_FREE(common_cpus_set);
if (i == *items) {
ary[*items] = set;
++*items;
@@ -98,13 +110,47 @@ void lscpu_sort_caches(struct lscpu_cache *caches, size_t n)
qsort(caches, n, sizeof(struct lscpu_cache), cmp_cache);
}
+/*
+ * Get the hotplug state number representing a completely online
+ * cpu from /sys/devices/system/cpu/hotplug/state
+ */
+static int get_online_state(struct path_cxt *sys)
+{
+ int hp_online_state_val, page_size, rc;
+ char *buf, *strp;
+
+ hp_online_state_val = -1;
+
+ /* sysfs text files have size = page size */
+ page_size = getpagesize();
+
+ buf = (char *)malloc(page_size);
+ if (!buf)
+ goto done;
+ rc = ul_path_readf_buffer(sys, buf, page_size, "hotplug/states");
+ if (rc <= 0)
+ goto done;
+
+ strp = strstr(buf, ": online");
+ if (!strp)
+ goto done;
+
+ strp--; /* get digits before ': online' */
+ while (strp >= buf && isdigit(*strp))
+ strp--;
+ hp_online_state_val = atoi(strp + 1);
+
+done:
+ free(buf);
+ return hp_online_state_val;
+}
/* Read topology for specified type */
static int cputype_read_topology(struct lscpu_cxt *cxt, struct lscpu_cputype *ct)
{
size_t i, npos;
struct path_cxt *sys;
- int nthreads = 0, sw_topo = 0;
+ int nthreads = 0, sw_topo = 0, rc, hp_state, hp_online_state;
FILE *fd;
sys = cxt->syscpu; /* /sys/devices/system/cpu/ */
@@ -112,6 +158,7 @@ static int cputype_read_topology(struct lscpu_cxt *cxt, struct lscpu_cputype *ct
DBG(TYPE, ul_debugobj(ct, "reading %s/%s/%s topology",
ct->vendor ?: "", ct->model ?: "", ct->modelname ?:""));
+ hp_online_state = get_online_state(sys);
for (i = 0; i < cxt->npossibles; i++) {
struct lscpu_cpu *cpu = cxt->cpus[i];
@@ -127,6 +174,15 @@ static int cputype_read_topology(struct lscpu_cxt *cxt, struct lscpu_cputype *ct
"cpu%d/topology/thread_siblings", num) != 0)
continue;
+ /*
+ * Ignore cpus which are not fully online.
+ * If hp_online_state is negative/zero or rc is negative,
+ * online state could not be read correctly, skip this check.
+ */
+ rc = ul_path_readf_s32(sys, &hp_state, "cpu%d/hotplug/state", num);
+ if (hp_online_state > 0 && rc >= 0 && hp_state != hp_online_state)
+ continue;
+
/* read topology maps */
ul_path_readf_cpuset(sys, &thread_siblings, cxt->maxcpus,
"cpu%d/topology/thread_siblings", num);
@@ -163,13 +219,13 @@ static int cputype_read_topology(struct lscpu_cxt *cxt, struct lscpu_cputype *ct
/* add to topology maps */
if (thread_siblings)
- add_cpuset_to_array(ct->coremaps, &ct->ncores, thread_siblings, cxt->setsize);
+ add_cpuset_to_array(ct->coremaps, &ct->ncores, thread_siblings, cxt->setsize, cxt->maxcpus);
if (core_siblings)
- add_cpuset_to_array(ct->socketmaps, &ct->nsockets, core_siblings, cxt->setsize);
+ add_cpuset_to_array(ct->socketmaps, &ct->nsockets, core_siblings, cxt->setsize, cxt->maxcpus);
if (book_siblings)
- add_cpuset_to_array(ct->bookmaps, &ct->nbooks, book_siblings, cxt->setsize);
+ add_cpuset_to_array(ct->bookmaps, &ct->nbooks, book_siblings, cxt->setsize, cxt->maxcpus);
if (drawer_siblings)
- add_cpuset_to_array(ct->drawermaps, &ct->ndrawers, drawer_siblings, cxt->setsize);
+ add_cpuset_to_array(ct->drawermaps, &ct->ndrawers, drawer_siblings, cxt->setsize, cxt->maxcpus);
}
base-commit: fda5dc760a0501554f079558ebd95a4f91fba7cd
--
2.46.0
^ permalink raw reply related
* Re: [RFC PATCH util-linux] text-utils: add bits command
From: Thomas Weißschuh @ 2024-10-18 6:53 UTC (permalink / raw)
To: Robin Jarry; +Cc: util-linux
In-Reply-To: <D4XJFK5WGV2R.244DMWY2H7ABM@ringo>
Oct 16, 2024 23:11:38 Robin Jarry <robin@jarry.cc>:
> Hi Thomas,
>
> Thomas Weißschuh, Oct 16, 2024 at 23:00:
>> On 2024-10-16 22:26:22+0200, Robin Jarry wrote:
>>> Add a new test utility to convert between bit masks in various formats.
>>
>> "test" or "text" utility?
>
> That was a typo, I meant "text" :)
>
>>> This can be handy to avoid parsing affinity masks in one's head and/or
>>> to interact with the kernel in a more human friendly way.
>>> This is a rewrite in C of the bits command from my linux-tools python
>>> package so that it can be more widely available.
>>> Here is an example:
>>> ~# cat /sys/kernel/debug/tracing/tracing_cpumask
>>> fffffff,ffffffff,ffffffff,ffffffff
>>> ~# bits -l ,$(cat /sys/kernel/debug/tracing/tracing_cpumask)
>>> 0-128
>>> ~# bits -g 58,59,120,123
>>> 9000000,00000000,0c000000,00000000
>>> ~# bits -g 58,59 > /sys/kernel/debug/tracing/tracing_cpumask
>>> ~# echo 1 > /sys/kernel/debug/tracing/tracing_on
>>> Add man page and basic tests.
>>> Link: https://git.sr.ht/~rjarry/linux-tools#bits
>>> Signed-off-by: Robin Jarry <robin@jarry.cc>
>>> ---
>>> bash-completion/bits | 21 ++
>>> meson.build | 11 ++
>>> tests/commands.sh | 1 +
>>> tests/expected/misc/bits | 0
>>> tests/expected/misc/bits-and | 1 +
>>> tests/expected/misc/bits-binary | 1 +
>>> tests/expected/misc/bits-default | 1 +
>>> tests/expected/misc/bits-grouped-mask | 1 +
>>> tests/expected/misc/bits-list | 1 +
>>> tests/expected/misc/bits-mask | 1 +
>>> tests/expected/misc/bits-not | 1 +
>>> tests/expected/misc/bits-or | 1 +
>>> tests/expected/misc/bits-overflow | 1 +
>>> tests/expected/misc/bits-parse-mask | 1 +
>>> tests/expected/misc/bits-parse-range | 1 +
>>> tests/expected/misc/bits-stdin | 1 +
>>> tests/expected/misc/bits-xor | 1 +
>>> tests/ts/misc/bits | 82 ++++++++
>>> text-utils/Makemodule.am | 6 +
>>> text-utils/bits.1.adoc | 147 ++++++++++++++
>>> text-utils/bits.c | 265 ++++++++++++++++++++++++++
>>> text-utils/meson.build | 4 +
>>> 22 files changed, 550 insertions(+)
>>> create mode 100644 bash-completion/bits
>>> create mode 100644 tests/expected/misc/bits
>>> create mode 100644 tests/expected/misc/bits-and
>>> create mode 100644 tests/expected/misc/bits-binary
>>> create mode 100644 tests/expected/misc/bits-default
>>> create mode 100644 tests/expected/misc/bits-grouped-mask
>>> create mode 100644 tests/expected/misc/bits-list
>>> create mode 100644 tests/expected/misc/bits-mask
>>> create mode 100644 tests/expected/misc/bits-not
>>> create mode 100644 tests/expected/misc/bits-or
>>> create mode 100644 tests/expected/misc/bits-overflow
>>> create mode 100644 tests/expected/misc/bits-parse-mask
>>> create mode 100644 tests/expected/misc/bits-parse-range
>>> create mode 100644 tests/expected/misc/bits-stdin
>>> create mode 100644 tests/expected/misc/bits-xor
>>> create mode 100755 tests/ts/misc/bits
>>> create mode 100644 text-utils/bits.1.adoc
>>> create mode 100644 text-utils/bits.c
>>
>> ...
>>
>>> +++ b/tests/ts/misc/bits
>>> @@ -0,0 +1,82 @@
>>> +#!/bin/bash
>>> +
>>> +#
>>> +# Copyright (c) 2024 Robin Jarry
>>> +#
>>> +# This file is part of util-linux.
>>> +#
>>> +# This file is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 2 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This file is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> +# GNU General Public License for more details.
>>
>> New files should use SPDX tags.
>
> Ack, will change in v2.
>
>>> diff --git a/text-utils/bits.c b/text-utils/bits.c
>>> new file mode 100644
>>> index 000000000000..6dd0db81a5de
>>> --- /dev/null
>>> +++ b/text-utils/bits.c
>>> @@ -0,0 +1,265 @@
>>> +/*
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * Copyright (c) 2024 Robin Jarry
>>> + *
>>> + * bits - convert bit masks from/to various formats
>>> + */
>>> +
>>> +#include <errno.h>
>>> +#include <getopt.h>
>>> +#include <sched.h>
>>> +#include <stdbool.h>
>>> +#include <stdint.h>
>>> +#include <stdio.h>
>>> +#include <string.h>
>>> +#include <unistd.h>
>>> +
>>> +#include "c.h"
>>> +#include "closestream.h"
>>> +#include "cpuset.h"
>>> +#include "nls.h"
>>> +#include "strutils.h"
>>> +#include "strv.h"
>>> +
>>> +typedef enum {
>>> + OP_OR,
>>> + OP_AND,
>>> + OP_NOT,
>>> + OP_XOR,
>>> +} bitvise_op_t;
>>
>> "bitwise"?
>> Also avoid using the _t suffix as it's reserved.
>> "enum bitwise_op" seems clear enough.
>
> Yes, bitwise is the correct spelling. I will probably remove that in v2 as you suggested below.
>
>>> +
>>> +static int parse_mask_or_list(const char *cmdline_arg, cpu_set_t *all_bits)
>>> +{
>>> + bitvise_op_t op = OP_OR;
>>> + cpu_set_t bits, copy;
>>
>> This seems very geared towards cpu masks.
>> What happens if a user wants to use it with very large non-CPU bitmasks,
>> as the tool is advertised as a generic utility?
>>
>> Call the utility "cpumask"?
>
> The default cpu_set_t size can hold up to 1024 bits (128 bytes). I assumed it was more than enough for common use cases. I added a basic test that checks that the program does not crash when specifying a too large mask.
> I also didn't want to call that tool "*mask" because it can also deal with lists. But I don't have a strong opinion on the matter.
I don't have an strong opinions on naming. Except that I'm bad at it.
The limitation should at least be documented.
(I didn't check of it already is)
>>> + char buf[BUFSIZ];
>>
>> Only use BUFSIZ in cases where the actual value does not matter,
>> for example loops.
>> On musl BUFSIZ == 1024 and this could not be enough fairly soon.
>
> Good point. I wasn't sure what is a reasonable maximum argument size. execve(2) says MAX_ARG_STRLEN (32 pages) which seems excessive. I could hardcode 8192 and be done with it. What do you think?
>
>>
>>> + char *arg = buf;
>>> +
>>> + /* copy to allow modifying the argument contents */
>>> + strlcpy(buf, cmdline_arg, sizeof(buf));
>>> + buf[sizeof(buf) - 1] = '\0';
>>
>> You can just use xstrdup().
>
> But then, I'd have to free it. I wanted to avoid this.
The CLI utilities of util-linux generally don't free memory if the tool will exit soon anyways.
>>
>>> +
>>> + /* strip optional operator first */
>>> + if (startswith(arg, "&")) {
>>> + op = OP_AND;
>>> + arg++;
>>> + } else if (startswith(arg, "^")) {
>>> + op = OP_XOR;
>>> + arg++;
>>> + } else if (startswith(arg, "~")) {
>>> + op = OP_NOT;
>>> + arg++;
>>> + } else if (startswith(arg, "|")) {
>>> + op = OP_OR;
>>> + arg++;
>>> + }
>>
>> The whole op enum could also be replaced by the actual character,
>> this is all in a single function.
>>
>> op = '&';
>>
>> ...
>>
>> switch (op) {
>> case '&':
>> }
>
> Good point. I'll do that in v2.
>
>>
>>> +
>>> + if (startswith(arg, ",") || startswith(arg, "0x")) {
>>> + if (cpumask_parse(arg, &bits, sizeof(bits)) < 0)
>>> + return -EINVAL;
>>
>> As this is a commandline tool you can call errx() here to kill the
>> process and provide better error information.
>> It would also make the call sequence slightly more simple.
>
> Ack.
>
>>
>>> + } else {
>>> + if (cpulist_parse(arg, &bits, sizeof(bits), 1) < 0)
>>> + return -EINVAL;
>>> + }
>>> +
>>> + switch (op) {
>>> + case OP_AND:
>>> + copy = *all_bits;
>>> + CPU_AND(all_bits, ©, &bits);
>>> + break;
>>> + case OP_OR:
>>> + copy = *all_bits;
>>> + CPU_OR(all_bits, ©, &bits);
>>> + break;
>>> + case OP_XOR:
>>> + copy = *all_bits;
>>> + CPU_XOR(all_bits, ©, &bits);
>>> + break;
>>> + case OP_NOT:
>>> + for (int i = 0; i < CPU_SETSIZE; i++) {
>>> + if (CPU_ISSET(i, &bits))
>>> + CPU_CLR(i, all_bits);
>>> + }
>>> + break;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +typedef enum {
>>> + MODE_BINARY,
>>> + MODE_GROUPED_MASK,
>>> + MODE_LIST,
>>> + MODE_MASK,
>>> +} output_mode_t;
>>> +
>>> +static void print_bits(cpu_set_t *bits, output_mode_t mode)
>>> +{
>>> + bool started = false;
>>> + char buf[BUFSIZ];
>>> + ssize_t n = 0;
>>> +
>>> + buf[0] = '\0';
>>
>> Is this necessary?
>
> It may have been at some point but it isn't anymore. I'll remove it.
>
>>
>>> +
>>> + switch (mode) {
>>> + case MODE_MASK:
>>> + cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
>>> +
>>> + /* strip leading zeroes */
>>> + while (buf[n] == '0')
>>> + n++;
>>> + if (buf[n] == '\0')
>>> + printf("0x0\n");
>>> + else
>>> + printf("0x%s\n", buf + n);
>>> + break;
>>> +
>>> + case MODE_GROUPED_MASK:
>>> + cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
>>> +
>>> + /* strip leading zeroes */
>>> + while (buf[n] == '0')
>>> + n++;
>>> +
>>> + while (buf[n] != '\0') {
>>> + if (started && (n % 8) == 0)
>>> + printf(",");
>>> + if (buf[n] != '0')
>>> + started = true;
>>> + printf("%c", buf[n]);
>>> + n++;
>>> + }
>>> + printf("\n");
>>> + break;
>>> +
>>> + case MODE_BINARY:
>>> + printf("0b");
>>> + for (n = CPU_SETSIZE; n >= 0; n--) {
>>> + if (started && ((n + 1) % 4) == 0)
>>> + printf("_");
>>> + if (CPU_ISSET(n, bits)) {
>>> + started = true;
>>> + printf("1");
>>> + } else if (started) {
>>> + printf("0");
>>> + }
>>> + }
>>> + printf("\n");
>>> + break;
>>> +
>>> + case MODE_LIST:
>>> + cpulist_create(buf, sizeof(buf), bits, sizeof(*bits));
>>
>> This could use the return values of cpulist_create() and
>> cpumask_create().
>
> Since I am printing into a stack buffer, is there a benefit?
Wouldn't the same hold true for a non-stack buffer?
Anyways, this shouldn't be important.
>
>>> + printf("%s\n", buf);
>>> + break;
>>> + }
>>> +
>>> +}
>>> +
>>> +static void __attribute__((__noreturn__)) usage(void)
>>> +{
>>> + fputs(USAGE_HEADER, stdout);
>>> + fprintf(stdout, _(" %s [options] [<mask_or_list>...]\n"), program_invocation_short_name);
>>> +
>>> + fputs(USAGE_SEPARATOR, stdout);
>>> + fputsln(_("Convert bit masks from/to various formats."), stdout);
>>> +
>>> + fputs(USAGE_ARGUMENTS, stdout);
>>> + fputsln(_(" <mask_or_list> A set of bits specified as a hex mask value (e.g. 0xeec2)\n"
>>> + " or as a comma-separated list of bit IDs.\n"
>>> + " If not specified, arguments will be read from stdin."), stdout);
>>> +
>>> + fputs(USAGE_OPTIONS, stdout);
>>> + fprintf(stdout, USAGE_HELP_OPTIONS(21));
>>> +
>>> + fputs(_("\nMode:\n"), stdout);
>>> + fputsln(_(" -m, --mask Print the combined args as a hex mask value (default).\n"), stdout);
>>> + fputsln(_(" -g, --grouped-mask Print the combined args as a hex mask value in 32bit\n"
>>> + " comma separated groups."), stdout);
>>> + fputsln(_(" -b, --binary Print the combined args as a binary mask value."), stdout);
>>> + fputsln(_(" -l, --list Print the combined args as a compressed list of bit IDs."), stdout);
>>> +
>>> + fputs(USAGE_SEPARATOR, stdout);
>>> + fprintf(stdout, USAGE_MAN_TAIL("bits(1)"));
>>> + exit(EXIT_SUCCESS);
>>> +}
>>> +
>>> +int main(int argc, char **argv)
>>> +{
>>> + output_mode_t mode = MODE_MASK;
>>> + char **stdin_lines = NULL;
>>> + cpu_set_t bits;
>>> + int c;
>>> +
>>> + static const struct option longopts[] = {
>>> + { "version", no_argument, NULL, 'V'},
>>> + { "help", no_argument, NULL, 'h'},
>>> + { "mask", no_argument, NULL, 'm'},
>>> + { "grouped-mask", no_argument, NULL, 'g'},
>>> + { "binary", no_argument, NULL, 'b'},
>>> + { "list", no_argument, NULL, 'l'},
>>> + { NULL, 0, NULL, 0 }
>>> + };
>>> +
>>> + setlocale(LC_ALL, "");
>>> + bindtextdomain(PACKAGE, LOCALEDIR);
>>> + textdomain(PACKAGE);
>>> + close_stdout_atexit();
>>> +
>>> + while ((c = getopt_long(argc, argv, "Vhmgbl", longopts, NULL)) != -1)
>>> + switch (c) {
>>> + case 'm':
>>> + mode = MODE_MASK;
>>> + break;
>>> + case 'g':
>>> + mode = MODE_GROUPED_MASK;
>>> + break;
>>> + case 'b':
>>> + mode = MODE_BINARY;
>>> + break;
>>> + case 'l':
>>> + mode = MODE_LIST;
>>> + break;
>>> + case 'V':
>>> + print_version(EXIT_SUCCESS);
>>> + case 'h':
>>> + usage();
>>> + default:
>>> + errtryhelp(EXIT_FAILURE);
>>> + }
>>> +
>>> + argc -= optind;
>>> + argv += optind;
>>> + if (argc == 0) {
>>> + /* no arguments provided, read lines from stdin */
>>> + char buf[BUFSIZ];
>>> +
>>> + while (fgets(buf, sizeof(buf), stdin)) {
>>> + /* strip LF, CR, CRLF, LFCR */
>>> + buf[strcspn(buf, "\r\n")] = '\0';
>>
>> rtrim_whitespace()?
>
> Ack.
>
>>
>>> + strv_push(&stdin_lines, strdup(buf));
>>> + }
>>> +
>>> + argc = strv_length(stdin_lines);
>>> + argv = stdin_lines;
>>> + }
>>> +
>>> + CPU_ZERO(&bits);
>>> +
>>> + for (; argc > 0; argc--, argv++)
>>> + if (parse_mask_or_list(*argv, &bits) < 0) {
>>> + fprintf(stderr, _("error: invalid argument: %s\n"), *argv);
>>> + errtryhelp(EXIT_FAILURE);
>>> + }
>>> +
>>> + strv_free(stdin_lines);
>>
>> No need to free this here. The process will exit anyways.
>
> I wanted to make ASAN happy. Is this not necessary?
As mentioned above, freeing memory at the end of the process lifetime isn't
really useful.
The kernel will need to clean it up anyways.
>>> +
>>> + print_bits(&bits, mode);
>>> +
>>> + return EXIT_SUCCESS;
>>> +}
>
> Thanks for the review!
^ permalink raw reply
* [PATCH util-linux v2] text-utils: add bits command
From: Robin Jarry @ 2024-10-16 21:57 UTC (permalink / raw)
To: util-linux; +Cc: Thomas Weißschuh
In-Reply-To: <20241016202621.2124554-2-robin@jarry.cc>
Add a new text utility to convert bit masks in various formats.
This can be handy to avoid parsing affinity masks in one's head and/or
to interact with the kernel in a more human friendly way. It is
a rewrite in C of the bits command from my linux-tools python package so
that it can be more widely available.
Here is an example:
~# cat /sys/kernel/debug/tracing/tracing_cpumask
ffffffff,ffffffff,ffffffff,ffffffff
~# bits -l ,$(cat /sys/kernel/debug/tracing/tracing_cpumask)
0-128
~# bits -g 58,59,120,123
9000000,00000000,0c000000,00000000
~# bits -g 58,59,120,123 > /sys/kernel/debug/tracing/tracing_cpumask
~# echo 1 > /sys/kernel/debug/tracing/tracing_on
Add man page and basic tests.
Link: https://git.sr.ht/~rjarry/linux-tools#bits
Signed-off-by: Robin Jarry <robin@jarry.cc>
---
Notes:
v2:
- Fixed grouped-mask parsing (e.g.: bits -l ,0c000000,00000000). Updated
unit tests to verify that it works.
- Changed code wrapping to keep arg strings in a single line.
- s/BUFSIZ/LINE_MAX/
- s/strdup/xstrdup/
- Removed unnecessary string copies.
- Added check for strv_push() return value.
- Removed "bitvise_op_t" enum and replaced with inline char values.
- Exit with explicit errors in parse_mask_or_list().
- Replaced license with SPDX in test file.
- Fixed configure.ac + Makemodule.am. Tested build with make.
- Fixed commit message example.
- Fixed typos.
.gitignore | 1 +
bash-completion/bits | 21 ++
configure.ac | 3 +
meson.build | 11 +
tests/commands.sh | 1 +
tests/expected/misc/bits | 0
tests/expected/misc/bits-and | 1 +
tests/expected/misc/bits-binary | 1 +
tests/expected/misc/bits-default | 1 +
tests/expected/misc/bits-grouped-mask | 1 +
tests/expected/misc/bits-list | 1 +
tests/expected/misc/bits-mask | 1 +
tests/expected/misc/bits-not | 1 +
tests/expected/misc/bits-or | 1 +
tests/expected/misc/bits-overflow | 1 +
tests/expected/misc/bits-parse-grouped-mask | 1 +
tests/expected/misc/bits-parse-mask | 1 +
tests/expected/misc/bits-parse-range | 1 +
tests/expected/misc/bits-stdin | 1 +
tests/expected/misc/bits-xor | 1 +
tests/ts/misc/bits | 81 +++++++
text-utils/Makemodule.am | 9 +-
text-utils/bits.1.adoc | 126 ++++++++++
text-utils/bits.c | 256 ++++++++++++++++++++
text-utils/meson.build | 4 +
25 files changed, 526 insertions(+), 1 deletion(-)
create mode 100644 bash-completion/bits
create mode 100644 tests/expected/misc/bits
create mode 100644 tests/expected/misc/bits-and
create mode 100644 tests/expected/misc/bits-binary
create mode 100644 tests/expected/misc/bits-default
create mode 100644 tests/expected/misc/bits-grouped-mask
create mode 100644 tests/expected/misc/bits-list
create mode 100644 tests/expected/misc/bits-mask
create mode 100644 tests/expected/misc/bits-not
create mode 100644 tests/expected/misc/bits-or
create mode 100644 tests/expected/misc/bits-overflow
create mode 100644 tests/expected/misc/bits-parse-grouped-mask
create mode 100644 tests/expected/misc/bits-parse-mask
create mode 100644 tests/expected/misc/bits-parse-range
create mode 100644 tests/expected/misc/bits-stdin
create mode 100644 tests/expected/misc/bits-xor
create mode 100755 tests/ts/misc/bits
create mode 100644 text-utils/bits.1.adoc
create mode 100644 text-utils/bits.c
diff --git a/.gitignore b/.gitignore
index 316f3cdcc76c..af5a1184472e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -76,6 +76,7 @@ ylwrap
#
/addpart
/agetty
+/bits
/build*/
/blkdiscard
/blkid
diff --git a/bash-completion/bits b/bash-completion/bits
new file mode 100644
index 000000000000..763e8669cfb2
--- /dev/null
+++ b/bash-completion/bits
@@ -0,0 +1,21 @@
+_bits_module()
+{
+ local cur prev OPTS
+ COMPREPLY=()
+ cur="${COMP_WORDS[COMP_CWORD]}"
+ prev="${COMP_WORDS[COMP_CWORD-1]}"
+ case $prev in
+ '-h'|'--help'|'-V'|'--version')
+ return 0
+ ;;
+ esac
+ case $cur in
+ -*)
+ OPTS="--version --help --mask --grouped-mask --bit --list"
+ COMPREPLY=( $(compgen -W "${OPTS[*]}" -- $cur) )
+ return 0
+ ;;
+ esac
+ return 0
+}
+complete -F _bits_module bits
diff --git a/configure.ac b/configure.ac
index b360e448ea95..6d7451f2f3b8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2233,6 +2233,9 @@ UL_REQUIRES_HAVE([scriptlive], [pty], [openpty function (libutil)])
AM_CONDITIONAL([BUILD_SCRIPTLIVE], [test "x$build_scriptlive" = xyes])
+UL_BUILD_INIT([bits], [yes])
+AM_CONDITIONAL([BUILD_BITS], [test "x$build_bits" = xyes])
+
UL_BUILD_INIT([col], [check])
UL_REQUIRES_COMPILE([col], [#include <limits.h>], [__GLIBC__], [building for glibc])
AM_CONDITIONAL([BUILD_COL], [test "x$build_col" = xyes])
diff --git a/meson.build b/meson.build
index a78ebd6b8934..db1c28f42bd3 100644
--- a/meson.build
+++ b/meson.build
@@ -1201,6 +1201,17 @@ endif
############################################################
+exe = executable(
+ 'bits',
+ bits_sources,
+ include_directories : includes,
+ link_with : lib_common,
+ install_dir : usrbin_exec_dir,
+ install : true)
+exes += exe
+manadocs += ['text-utils/bits.1.adoc']
+bashcompletions += ['bits']
+
if is_glibc
exe = executable(
'col',
diff --git a/tests/commands.sh b/tests/commands.sh
index 9eef92ccbb72..4402e38b4118 100644
--- a/tests/commands.sh
+++ b/tests/commands.sh
@@ -64,6 +64,7 @@ TS_HELPER_TIMEUTILS="${ts_helpersdir}test_timeutils"
# paths to commands
TS_CMD_ADDPART=${TS_CMD_ADDPART:-"${ts_commandsdir}addpart"}
TS_CMD_DELPART=${TS_CMD_DELPART:-"${ts_commandsdir}delpart"}
+TS_CMD_BITS=${TS_CMD_BITS-"${ts_commandsdir}bits"}
TS_CMD_BLKDISCARD=${TS_CMD_BLKID-"${ts_commandsdir}blkdiscard"}
TS_CMD_BLKID=${TS_CMD_BLKID-"${ts_commandsdir}blkid"}
TS_CMD_CAL=${TS_CMD_CAL-"${ts_commandsdir}cal"}
diff --git a/tests/expected/misc/bits b/tests/expected/misc/bits
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/expected/misc/bits-and b/tests/expected/misc/bits-and
new file mode 100644
index 000000000000..1d8ffee8c97a
--- /dev/null
+++ b/tests/expected/misc/bits-and
@@ -0,0 +1 @@
+75-100
diff --git a/tests/expected/misc/bits-binary b/tests/expected/misc/bits-binary
new file mode 100644
index 000000000000..ba7b220e9354
--- /dev/null
+++ b/tests/expected/misc/bits-binary
@@ -0,0 +1 @@
+0b1_0000_0000_0010_0000_0000_0100_0000_0000_1000_0000_0000
diff --git a/tests/expected/misc/bits-default b/tests/expected/misc/bits-default
new file mode 100644
index 000000000000..a56502d4c880
--- /dev/null
+++ b/tests/expected/misc/bits-default
@@ -0,0 +1 @@
+0x100200400800
diff --git a/tests/expected/misc/bits-grouped-mask b/tests/expected/misc/bits-grouped-mask
new file mode 100644
index 000000000000..427fc5c2a6ca
--- /dev/null
+++ b/tests/expected/misc/bits-grouped-mask
@@ -0,0 +1 @@
+1002,00400800
diff --git a/tests/expected/misc/bits-list b/tests/expected/misc/bits-list
new file mode 100644
index 000000000000..7511e5378ea4
--- /dev/null
+++ b/tests/expected/misc/bits-list
@@ -0,0 +1 @@
+11,22,33,44
diff --git a/tests/expected/misc/bits-mask b/tests/expected/misc/bits-mask
new file mode 100644
index 000000000000..a56502d4c880
--- /dev/null
+++ b/tests/expected/misc/bits-mask
@@ -0,0 +1 @@
+0x100200400800
diff --git a/tests/expected/misc/bits-not b/tests/expected/misc/bits-not
new file mode 100644
index 000000000000..2487fcfecb22
--- /dev/null
+++ b/tests/expected/misc/bits-not
@@ -0,0 +1 @@
+50-74
diff --git a/tests/expected/misc/bits-or b/tests/expected/misc/bits-or
new file mode 100644
index 000000000000..753370ac5c3b
--- /dev/null
+++ b/tests/expected/misc/bits-or
@@ -0,0 +1 @@
+50-150
diff --git a/tests/expected/misc/bits-overflow b/tests/expected/misc/bits-overflow
new file mode 100644
index 000000000000..9982566dc094
--- /dev/null
+++ b/tests/expected/misc/bits-overflow
@@ -0,0 +1 @@
+0x0
diff --git a/tests/expected/misc/bits-parse-grouped-mask b/tests/expected/misc/bits-parse-grouped-mask
new file mode 100644
index 000000000000..3ed39af60315
--- /dev/null
+++ b/tests/expected/misc/bits-parse-grouped-mask
@@ -0,0 +1 @@
+58,59,120,123
diff --git a/tests/expected/misc/bits-parse-mask b/tests/expected/misc/bits-parse-mask
new file mode 100644
index 000000000000..59dd4b4c1696
--- /dev/null
+++ b/tests/expected/misc/bits-parse-mask
@@ -0,0 +1 @@
+1,3,6,7,9,11,14-16,18,19,21,23-25,27
diff --git a/tests/expected/misc/bits-parse-range b/tests/expected/misc/bits-parse-range
new file mode 100644
index 000000000000..5afeb0332629
--- /dev/null
+++ b/tests/expected/misc/bits-parse-range
@@ -0,0 +1 @@
+7fffff,ffffffff,ffffffff,fffc0000,00000000
diff --git a/tests/expected/misc/bits-stdin b/tests/expected/misc/bits-stdin
new file mode 100644
index 000000000000..00ff99b2a15c
--- /dev/null
+++ b/tests/expected/misc/bits-stdin
@@ -0,0 +1 @@
+11,33,44
diff --git a/tests/expected/misc/bits-xor b/tests/expected/misc/bits-xor
new file mode 100644
index 000000000000..55289438dc57
--- /dev/null
+++ b/tests/expected/misc/bits-xor
@@ -0,0 +1 @@
+50-74,101-150
diff --git a/tests/ts/misc/bits b/tests/ts/misc/bits
new file mode 100755
index 000000000000..ebdf816713f0
--- /dev/null
+++ b/tests/ts/misc/bits
@@ -0,0 +1,81 @@
+#!/bin/bash
+
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# Copyright (c) 2024 Robin Jarry
+#
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="bits"
+
+. "$TS_TOPDIR"/functions.sh
+ts_init "$*"
+
+ts_check_test_command "$TS_CMD_BITS"
+ts_cd "$TS_OUTDIR"
+
+ts_init_subtest "default"
+$TS_CMD_BITS 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "mask"
+$TS_CMD_BITS --mask 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "grouped-mask"
+$TS_CMD_BITS --grouped-mask 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "list"
+$TS_CMD_BITS --list 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "binary"
+$TS_CMD_BITS --binary 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "overflow"
+$TS_CMD_BITS 129837984734 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "parse-mask"
+$TS_CMD_BITS -l 0x0badcaca >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "parse-range"
+$TS_CMD_BITS -g 50-100 75-150 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "parse-grouped-mask"
+$TS_CMD_BITS -l ,9000000,00000000,0c000000,00000000 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "or"
+$TS_CMD_BITS -l 50-100 '|75-150' >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "and"
+$TS_CMD_BITS -l 50-100 '&75-150' >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "xor"
+$TS_CMD_BITS -l 50-100 '^75-150' >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "not"
+$TS_CMD_BITS -l 50-100 '~75-150' >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "stdin"
+{
+ echo 11,22,33,44
+ echo ^22
+} | $TS_CMD_BITS --list >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_finalize
diff --git a/text-utils/Makemodule.am b/text-utils/Makemodule.am
index af1bf0238664..b135e3a02819 100644
--- a/text-utils/Makemodule.am
+++ b/text-utils/Makemodule.am
@@ -1,3 +1,11 @@
+if BUILD_BITS
+usrbin_exec_PROGRAMS += bits
+MANPAGES += text-utils/bits.1
+dist_noinst_DATA += text-utils/bits.1.adoc
+bits_SOURCES = text-utils/bits.c
+bits_LDADD = $(LDADD) libcommon.la
+endif
+
if BUILD_COL
usrbin_exec_PROGRAMS += col
MANPAGES += text-utils/col.1
@@ -105,4 +113,3 @@ test_more_CFLAGS = -DTEST_PROGRAM $(more_CFLAGS)
test_more_LDADD = $(more_LDADD)
endif # BUILD_MORE
-
diff --git a/text-utils/bits.1.adoc b/text-utils/bits.1.adoc
new file mode 100644
index 000000000000..b6de8c5936f0
--- /dev/null
+++ b/text-utils/bits.1.adoc
@@ -0,0 +1,126 @@
+//po4a: entry man manual
+////
+
+SPDX-License-Identifier: GPL-2.0-or-later
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+Copyright (c) 2024 Robin Jarry
+
+////
+= bits(1)
+:doctype: manpage
+:man manual: User Commands
+:man source: util-linux {release-version}
+:page-layout: base
+:command: bits
+
+== NAME
+
+bits - convert bit masks from/to various formats
+
+== SYNOPSIS
+
+*bits* [*-h*] [*-V*] [_<MODE>_] [_<MASK_OR_LIST>_...]
+
+== DESCRIPTION
+
+The *bits* utility converts bit masks into various formats. It supports
+combining multiple masks together using bitwise operations.
+
+== POSITIONAL ARGUMENTS
+
+_<MASK_OR_LIST>_::
+A set of bits specified as a hexadecimal mask value (e.g. _0xeec2_) or as
+a comma-separated list of bit IDs.
+
+If no argument is specified, the sets of bits will be read from standard input;
+one group per line.
+
+Consecutive ids can be compressed as ranges (e.g. _5,6,7,8,9,10_ -> _5-10_).
+
+Optionally, if an argument starts with a comma, it will be parsed as a single
+hexadecimal mask split in 32bit groups (e.g. _,00014000,00000000,00020000_ ->
+_17,78,80_).
+
+By default all groups will be OR'ed together. If a group has one of the
+following prefixes, it will be combined with the resulting mask using
+a different binary operation:
+
+**&**__<MASK_OR_LIST>__::
+The group will be combined with a binary AND operation. I.e. all bits that are
+set to 1 in the group AND the combined groups so far will be preserved to 1.
+All other bits will be reset to 0.
+
+**^**__<MASK_OR_LIST>__::
+The group will be combined with a binary XOR operation. I.e. all bits that are
+set to 1 in the group AND to 0 the combined groups so far (or the other way
+around) will be set to 1. Bits that are both to 1 or both to 0 will be reset to
+0.
+
+**~**__<MASK_OR_LIST>__::
+All bits set to 1 in the group will be cleared (reset to 0) in the combined
+groups so far.
+
+== OPTIONS
+
+include::man-common/help-version.adoc[]
+
+== CONVERSION MODE
+
+One of the following conversion modes can be specified. If not specified, it
+defaults to *-m*, *--mask*.
+
+*-m*, *--mask*::
+Print the combined args as a hexadecimal mask value (default).
+
+*-g*, *--grouped-mask*::
+Print the combined args as a hexadecimal mask value in 32bit comma separated
+groups.
+
+*-b*, *--binary*::
+Print the combined args as a binary mask value.
+
+*-l*, *--list*::
+Print the combined args as a list of bit IDs. Consecutive IDs are compressed as
+ranges.
+
+== EXAMPLES
+
+....
+~$ bits --mask 4,5-8 16,30
+0x400101f0
+
+~$ bits --list 0xeec2
+1,6,7,9-11,13-15
+
+~$ bits --binary 4,5-8 16,30
+0b100_0000_0000_0001_0000_0001_1111_0000
+
+~$ bits --list ,00300000,03000000,30000003
+0,1,28,29,56,57,84,85
+
+~$ bits --list 1,2,3,4 ~3-10
+1,2
+
+~$ bits --list 1,2,3,4 ^3-10
+1,2,5-10
+
+~$ bits --grouped-mask 2,22,74,79
+8400,00000000,00400004
+....
+
+== AUTHORS
+
+Robin Jarry.
+
+include::man-common/bugreports.adoc[]
+
+include::man-common/footer.adoc[]
+
+ifdef::translation[]
+include::man-common/translation.adoc[]
+endif::[]
diff --git a/text-utils/bits.c b/text-utils/bits.c
new file mode 100644
index 000000000000..00a9ce57a4dc
--- /dev/null
+++ b/text-utils/bits.c
@@ -0,0 +1,256 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Copyright (c) 2024 Robin Jarry
+ *
+ * bits - convert bit masks from/to various formats
+ */
+
+#include <errno.h>
+#include <getopt.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "c.h"
+#include "closestream.h"
+#include "cpuset.h"
+#include "nls.h"
+#include "strutils.h"
+#include "strv.h"
+#include "xalloc.h"
+
+static void parse_mask_or_list(const char *cmdline_arg, cpu_set_t *all_bits)
+{
+ char bitwise_op = '|';
+ cpu_set_t bits, copy;
+ const char *arg;
+
+ arg = cmdline_arg;
+
+ /* strip optional operator first */
+ if (startswith(arg, "&")) {
+ bitwise_op = '&';
+ arg++;
+ } else if (startswith(arg, "^")) {
+ bitwise_op = '^';
+ arg++;
+ } else if (startswith(arg, "~")) {
+ bitwise_op = '~';
+ arg++;
+ } else if (startswith(arg, "|")) {
+ arg++;
+ }
+
+ if (startswith(arg, ",") || startswith(arg, "0x")) {
+ if (startswith(arg, ","))
+ arg++;
+ if (cpumask_parse(arg, &bits, sizeof(bits)) < 0)
+ errx(EXIT_FAILURE, _("error: invalid bit mask: %s"), cmdline_arg);
+ } else {
+ if (cpulist_parse(arg, &bits, sizeof(bits), 1) < 0)
+ errx(EXIT_FAILURE, _("error: invalid bit list: %s"), cmdline_arg);
+ }
+
+ switch (bitwise_op) {
+ case '&':
+ copy = *all_bits;
+ CPU_AND(all_bits, ©, &bits);
+ break;
+ case '|':
+ copy = *all_bits;
+ CPU_OR(all_bits, ©, &bits);
+ break;
+ case '^':
+ copy = *all_bits;
+ CPU_XOR(all_bits, ©, &bits);
+ break;
+ case '~':
+ for (int i = 0; i < CPU_SETSIZE; i++) {
+ if (CPU_ISSET(i, &bits))
+ CPU_CLR(i, all_bits);
+ }
+ break;
+ }
+}
+
+enum output_mode {
+ MODE_BINARY,
+ MODE_GROUPED_MASK,
+ MODE_LIST,
+ MODE_MASK,
+};
+
+static void print_bits(cpu_set_t *bits, enum output_mode mode)
+{
+ bool started = false;
+ char buf[LINE_MAX];
+ ssize_t n = 0;
+
+ switch (mode) {
+ case MODE_MASK:
+ cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
+
+ /* strip leading zeroes */
+ while (buf[n] == '0')
+ n++;
+ if (buf[n] == '\0')
+ printf("0x0\n");
+ else
+ printf("0x%s\n", buf + n);
+ break;
+
+ case MODE_GROUPED_MASK:
+ cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
+
+ /* strip leading zeroes */
+ while (buf[n] == '0')
+ n++;
+
+ while (buf[n] != '\0') {
+ if (started && (n % 8) == 0)
+ printf(",");
+ if (buf[n] != '0')
+ started = true;
+ printf("%c", buf[n]);
+ n++;
+ }
+ printf("\n");
+ break;
+
+ case MODE_BINARY:
+ printf("0b");
+ for (n = CPU_SETSIZE; n >= 0; n--) {
+ if (started && ((n + 1) % 4) == 0)
+ printf("_");
+ if (CPU_ISSET(n, bits)) {
+ started = true;
+ printf("1");
+ } else if (started) {
+ printf("0");
+ }
+ }
+ printf("\n");
+ break;
+
+ case MODE_LIST:
+ cpulist_create(buf, sizeof(buf), bits, sizeof(*bits));
+ printf("%s\n", buf);
+ break;
+ }
+}
+
+static void __attribute__((__noreturn__)) usage(void)
+{
+ fputs(USAGE_HEADER, stdout);
+ fprintf(stdout, _(" %s [options] [<mask_or_list>...]\n"),
+ program_invocation_short_name);
+
+ fputs(USAGE_SEPARATOR, stdout);
+ fputsln(_("Convert bit masks from/to various formats."), stdout);
+
+ fputs(USAGE_ARGUMENTS, stdout);
+ fputsln(_(" <mask_or_list> A set of bits specified as a hex mask value (e.g. 0xeec2)\n"
+ " or as a comma-separated list of bit IDs."),
+ stdout);
+ fputs(USAGE_SEPARATOR, stdout);
+ fputsln(_(" If not specified, arguments will be read from stdin."),
+ stdout);
+
+ fputs(USAGE_OPTIONS, stdout);
+ fprintf(stdout, USAGE_HELP_OPTIONS(21));
+
+ fputs(_("\nMode:\n"), stdout);
+ fputsln(_(" -m, --mask Print the combined args as a hex mask value (default)."),
+ stdout);
+ fputsln(_(" -g, --grouped-mask Print the combined args as a hex mask value in 32bit\n"
+ " comma separated groups."), stdout);
+ fputsln(_(" -b, --binary Print the combined args as a binary mask value."),
+ stdout);
+ fputsln(_(" -l, --list Print the combined args as a compressed list of bit IDs."),
+ stdout);
+
+ fprintf(stdout, USAGE_MAN_TAIL("bits(1)"));
+ exit(EXIT_SUCCESS);
+}
+
+int main(int argc, char **argv)
+{
+ enum output_mode mode = MODE_MASK;
+ char **stdin_lines = NULL;
+ cpu_set_t bits;
+ int c;
+
+ static const struct option longopts[] = {
+ { "version", no_argument, NULL, 'V'},
+ { "help", no_argument, NULL, 'h'},
+ { "mask", no_argument, NULL, 'm'},
+ { "grouped-mask", no_argument, NULL, 'g'},
+ { "binary", no_argument, NULL, 'b'},
+ { "list", no_argument, NULL, 'l'},
+ { NULL, 0, NULL, 0 }
+ };
+
+ setlocale(LC_ALL, "");
+ bindtextdomain(PACKAGE, LOCALEDIR);
+ textdomain(PACKAGE);
+ close_stdout_atexit();
+
+ while ((c = getopt_long(argc, argv, "Vhmgbl", longopts, NULL)) != -1)
+ switch (c) {
+ case 'm':
+ mode = MODE_MASK;
+ break;
+ case 'g':
+ mode = MODE_GROUPED_MASK;
+ break;
+ case 'b':
+ mode = MODE_BINARY;
+ break;
+ case 'l':
+ mode = MODE_LIST;
+ break;
+ case 'V':
+ print_version(EXIT_SUCCESS);
+ case 'h':
+ usage();
+ default:
+ errtryhelp(EXIT_FAILURE);
+ }
+
+ argc -= optind;
+ argv += optind;
+ if (argc == 0) {
+ /* no arguments provided, read lines from stdin */
+ char buf[LINE_MAX];
+
+ while (fgets(buf, sizeof(buf), stdin)) {
+ /* strip LF, CR, CRLF, LFCR */
+ rtrim_whitespace((unsigned char *)buf);
+ if (strv_push(&stdin_lines, xstrdup(buf)) < 0)
+ errx(EXIT_FAILURE, _("cannot allocate memory"));
+ }
+
+ argc = strv_length(stdin_lines);
+ argv = stdin_lines;
+ }
+
+ CPU_ZERO(&bits);
+
+ for (; argc > 0; argc--, argv++)
+ parse_mask_or_list(*argv, &bits);
+
+ strv_free(stdin_lines);
+
+ print_bits(&bits, mode);
+
+ return EXIT_SUCCESS;
+}
diff --git a/text-utils/meson.build b/text-utils/meson.build
index f3b25d382160..4a25fa478328 100644
--- a/text-utils/meson.build
+++ b/text-utils/meson.build
@@ -1,3 +1,7 @@
+bits_sources = files(
+ 'bits.c',
+)
+
col_sources = files(
'col.c',
)
--
2.47.0
^ permalink raw reply related
* Re: [RFC PATCH util-linux] text-utils: add bits command
From: Robin Jarry @ 2024-10-16 21:11 UTC (permalink / raw)
To: Thomas Weißschuh; +Cc: util-linux
In-Reply-To: <0792f071-786b-4cb2-9733-2b10c98c20f2@t-8ch.de>
Hi Thomas,
Thomas Weißschuh, Oct 16, 2024 at 23:00:
> On 2024-10-16 22:26:22+0200, Robin Jarry wrote:
>> Add a new test utility to convert between bit masks in various formats.
>
> "test" or "text" utility?
That was a typo, I meant "text" :)
>> This can be handy to avoid parsing affinity masks in one's head and/or
>> to interact with the kernel in a more human friendly way.
>>
>> This is a rewrite in C of the bits command from my linux-tools python
>> package so that it can be more widely available.
>>
>> Here is an example:
>>
>> ~# cat /sys/kernel/debug/tracing/tracing_cpumask
>> fffffff,ffffffff,ffffffff,ffffffff
>> ~# bits -l ,$(cat /sys/kernel/debug/tracing/tracing_cpumask)
>> 0-128
>> ~# bits -g 58,59,120,123
>> 9000000,00000000,0c000000,00000000
>> ~# bits -g 58,59 > /sys/kernel/debug/tracing/tracing_cpumask
>> ~# echo 1 > /sys/kernel/debug/tracing/tracing_on
>>
>> Add man page and basic tests.
>>
>> Link: https://git.sr.ht/~rjarry/linux-tools#bits
>> Signed-off-by: Robin Jarry <robin@jarry.cc>
>> ---
>> bash-completion/bits | 21 ++
>> meson.build | 11 ++
>> tests/commands.sh | 1 +
>> tests/expected/misc/bits | 0
>> tests/expected/misc/bits-and | 1 +
>> tests/expected/misc/bits-binary | 1 +
>> tests/expected/misc/bits-default | 1 +
>> tests/expected/misc/bits-grouped-mask | 1 +
>> tests/expected/misc/bits-list | 1 +
>> tests/expected/misc/bits-mask | 1 +
>> tests/expected/misc/bits-not | 1 +
>> tests/expected/misc/bits-or | 1 +
>> tests/expected/misc/bits-overflow | 1 +
>> tests/expected/misc/bits-parse-mask | 1 +
>> tests/expected/misc/bits-parse-range | 1 +
>> tests/expected/misc/bits-stdin | 1 +
>> tests/expected/misc/bits-xor | 1 +
>> tests/ts/misc/bits | 82 ++++++++
>> text-utils/Makemodule.am | 6 +
>> text-utils/bits.1.adoc | 147 ++++++++++++++
>> text-utils/bits.c | 265 ++++++++++++++++++++++++++
>> text-utils/meson.build | 4 +
>> 22 files changed, 550 insertions(+)
>> create mode 100644 bash-completion/bits
>> create mode 100644 tests/expected/misc/bits
>> create mode 100644 tests/expected/misc/bits-and
>> create mode 100644 tests/expected/misc/bits-binary
>> create mode 100644 tests/expected/misc/bits-default
>> create mode 100644 tests/expected/misc/bits-grouped-mask
>> create mode 100644 tests/expected/misc/bits-list
>> create mode 100644 tests/expected/misc/bits-mask
>> create mode 100644 tests/expected/misc/bits-not
>> create mode 100644 tests/expected/misc/bits-or
>> create mode 100644 tests/expected/misc/bits-overflow
>> create mode 100644 tests/expected/misc/bits-parse-mask
>> create mode 100644 tests/expected/misc/bits-parse-range
>> create mode 100644 tests/expected/misc/bits-stdin
>> create mode 100644 tests/expected/misc/bits-xor
>> create mode 100755 tests/ts/misc/bits
>> create mode 100644 text-utils/bits.1.adoc
>> create mode 100644 text-utils/bits.c
>
> ...
>
>> +++ b/tests/ts/misc/bits
>> @@ -0,0 +1,82 @@
>> +#!/bin/bash
>> +
>> +#
>> +# Copyright (c) 2024 Robin Jarry
>> +#
>> +# This file is part of util-linux.
>> +#
>> +# This file is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This file is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> +# GNU General Public License for more details.
>
> New files should use SPDX tags.
Ack, will change in v2.
>> diff --git a/text-utils/bits.c b/text-utils/bits.c
>> new file mode 100644
>> index 000000000000..6dd0db81a5de
>> --- /dev/null
>> +++ b/text-utils/bits.c
>> @@ -0,0 +1,265 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * Copyright (c) 2024 Robin Jarry
>> + *
>> + * bits - convert bit masks from/to various formats
>> + */
>> +
>> +#include <errno.h>
>> +#include <getopt.h>
>> +#include <sched.h>
>> +#include <stdbool.h>
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +
>> +#include "c.h"
>> +#include "closestream.h"
>> +#include "cpuset.h"
>> +#include "nls.h"
>> +#include "strutils.h"
>> +#include "strv.h"
>> +
>> +typedef enum {
>> + OP_OR,
>> + OP_AND,
>> + OP_NOT,
>> + OP_XOR,
>> +} bitvise_op_t;
>
> "bitwise"?
> Also avoid using the _t suffix as it's reserved.
> "enum bitwise_op" seems clear enough.
Yes, bitwise is the correct spelling. I will probably remove that in v2
as you suggested below.
>> +
>> +static int parse_mask_or_list(const char *cmdline_arg, cpu_set_t *all_bits)
>> +{
>> + bitvise_op_t op = OP_OR;
>> + cpu_set_t bits, copy;
>
> This seems very geared towards cpu masks.
> What happens if a user wants to use it with very large non-CPU bitmasks,
> as the tool is advertised as a generic utility?
>
> Call the utility "cpumask"?
The default cpu_set_t size can hold up to 1024 bits (128 bytes).
I assumed it was more than enough for common use cases. I added a basic
test that checks that the program does not crash when specifying a too
large mask.
I also didn't want to call that tool "*mask" because it can also deal
with lists. But I don't have a strong opinion on the matter.
>
>> + char buf[BUFSIZ];
>
> Only use BUFSIZ in cases where the actual value does not matter,
> for example loops.
> On musl BUFSIZ == 1024 and this could not be enough fairly soon.
Good point. I wasn't sure what is a reasonable maximum argument size.
execve(2) says MAX_ARG_STRLEN (32 pages) which seems excessive. I could
hardcode 8192 and be done with it. What do you think?
>
>> + char *arg = buf;
>> +
>> + /* copy to allow modifying the argument contents */
>> + strlcpy(buf, cmdline_arg, sizeof(buf));
>> + buf[sizeof(buf) - 1] = '\0';
>
> You can just use xstrdup().
But then, I'd have to free it. I wanted to avoid this.
>
>> +
>> + /* strip optional operator first */
>> + if (startswith(arg, "&")) {
>> + op = OP_AND;
>> + arg++;
>> + } else if (startswith(arg, "^")) {
>> + op = OP_XOR;
>> + arg++;
>> + } else if (startswith(arg, "~")) {
>> + op = OP_NOT;
>> + arg++;
>> + } else if (startswith(arg, "|")) {
>> + op = OP_OR;
>> + arg++;
>> + }
>
> The whole op enum could also be replaced by the actual character,
> this is all in a single function.
>
> op = '&';
>
> ...
>
> switch (op) {
> case '&':
> }
Good point. I'll do that in v2.
>
>> +
>> + if (startswith(arg, ",") || startswith(arg, "0x")) {
>> + if (cpumask_parse(arg, &bits, sizeof(bits)) < 0)
>> + return -EINVAL;
>
> As this is a commandline tool you can call errx() here to kill the
> process and provide better error information.
> It would also make the call sequence slightly more simple.
Ack.
>
>> + } else {
>> + if (cpulist_parse(arg, &bits, sizeof(bits), 1) < 0)
>> + return -EINVAL;
>> + }
>> +
>> + switch (op) {
>> + case OP_AND:
>> + copy = *all_bits;
>> + CPU_AND(all_bits, ©, &bits);
>> + break;
>> + case OP_OR:
>> + copy = *all_bits;
>> + CPU_OR(all_bits, ©, &bits);
>> + break;
>> + case OP_XOR:
>> + copy = *all_bits;
>> + CPU_XOR(all_bits, ©, &bits);
>> + break;
>> + case OP_NOT:
>> + for (int i = 0; i < CPU_SETSIZE; i++) {
>> + if (CPU_ISSET(i, &bits))
>> + CPU_CLR(i, all_bits);
>> + }
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +typedef enum {
>> + MODE_BINARY,
>> + MODE_GROUPED_MASK,
>> + MODE_LIST,
>> + MODE_MASK,
>> +} output_mode_t;
>> +
>> +static void print_bits(cpu_set_t *bits, output_mode_t mode)
>> +{
>> + bool started = false;
>> + char buf[BUFSIZ];
>> + ssize_t n = 0;
>> +
>> + buf[0] = '\0';
>
> Is this necessary?
It may have been at some point but it isn't anymore. I'll remove it.
>
>> +
>> + switch (mode) {
>> + case MODE_MASK:
>> + cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
>> +
>> + /* strip leading zeroes */
>> + while (buf[n] == '0')
>> + n++;
>> + if (buf[n] == '\0')
>> + printf("0x0\n");
>> + else
>> + printf("0x%s\n", buf + n);
>> + break;
>> +
>> + case MODE_GROUPED_MASK:
>> + cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
>> +
>> + /* strip leading zeroes */
>> + while (buf[n] == '0')
>> + n++;
>> +
>> + while (buf[n] != '\0') {
>> + if (started && (n % 8) == 0)
>> + printf(",");
>> + if (buf[n] != '0')
>> + started = true;
>> + printf("%c", buf[n]);
>> + n++;
>> + }
>> + printf("\n");
>> + break;
>> +
>> + case MODE_BINARY:
>> + printf("0b");
>> + for (n = CPU_SETSIZE; n >= 0; n--) {
>> + if (started && ((n + 1) % 4) == 0)
>> + printf("_");
>> + if (CPU_ISSET(n, bits)) {
>> + started = true;
>> + printf("1");
>> + } else if (started) {
>> + printf("0");
>> + }
>> + }
>> + printf("\n");
>> + break;
>> +
>> + case MODE_LIST:
>> + cpulist_create(buf, sizeof(buf), bits, sizeof(*bits));
>
> This could use the return values of cpulist_create() and
> cpumask_create().
Since I am printing into a stack buffer, is there a benefit?
>> + printf("%s\n", buf);
>> + break;
>> + }
>> +
>> +}
>> +
>> +static void __attribute__((__noreturn__)) usage(void)
>> +{
>> + fputs(USAGE_HEADER, stdout);
>> + fprintf(stdout, _(" %s [options] [<mask_or_list>...]\n"), program_invocation_short_name);
>> +
>> + fputs(USAGE_SEPARATOR, stdout);
>> + fputsln(_("Convert bit masks from/to various formats."), stdout);
>> +
>> + fputs(USAGE_ARGUMENTS, stdout);
>> + fputsln(_(" <mask_or_list> A set of bits specified as a hex mask value (e.g. 0xeec2)\n"
>> + " or as a comma-separated list of bit IDs.\n"
>> + " If not specified, arguments will be read from stdin."), stdout);
>> +
>> + fputs(USAGE_OPTIONS, stdout);
>> + fprintf(stdout, USAGE_HELP_OPTIONS(21));
>> +
>> + fputs(_("\nMode:\n"), stdout);
>> + fputsln(_(" -m, --mask Print the combined args as a hex mask value (default).\n"), stdout);
>> + fputsln(_(" -g, --grouped-mask Print the combined args as a hex mask value in 32bit\n"
>> + " comma separated groups."), stdout);
>> + fputsln(_(" -b, --binary Print the combined args as a binary mask value."), stdout);
>> + fputsln(_(" -l, --list Print the combined args as a compressed list of bit IDs."), stdout);
>> +
>> + fputs(USAGE_SEPARATOR, stdout);
>> + fprintf(stdout, USAGE_MAN_TAIL("bits(1)"));
>> + exit(EXIT_SUCCESS);
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> + output_mode_t mode = MODE_MASK;
>> + char **stdin_lines = NULL;
>> + cpu_set_t bits;
>> + int c;
>> +
>> + static const struct option longopts[] = {
>> + { "version", no_argument, NULL, 'V'},
>> + { "help", no_argument, NULL, 'h'},
>> + { "mask", no_argument, NULL, 'm'},
>> + { "grouped-mask", no_argument, NULL, 'g'},
>> + { "binary", no_argument, NULL, 'b'},
>> + { "list", no_argument, NULL, 'l'},
>> + { NULL, 0, NULL, 0 }
>> + };
>> +
>> + setlocale(LC_ALL, "");
>> + bindtextdomain(PACKAGE, LOCALEDIR);
>> + textdomain(PACKAGE);
>> + close_stdout_atexit();
>> +
>> + while ((c = getopt_long(argc, argv, "Vhmgbl", longopts, NULL)) != -1)
>> + switch (c) {
>> + case 'm':
>> + mode = MODE_MASK;
>> + break;
>> + case 'g':
>> + mode = MODE_GROUPED_MASK;
>> + break;
>> + case 'b':
>> + mode = MODE_BINARY;
>> + break;
>> + case 'l':
>> + mode = MODE_LIST;
>> + break;
>> + case 'V':
>> + print_version(EXIT_SUCCESS);
>> + case 'h':
>> + usage();
>> + default:
>> + errtryhelp(EXIT_FAILURE);
>> + }
>> +
>> + argc -= optind;
>> + argv += optind;
>> + if (argc == 0) {
>> + /* no arguments provided, read lines from stdin */
>> + char buf[BUFSIZ];
>> +
>> + while (fgets(buf, sizeof(buf), stdin)) {
>> + /* strip LF, CR, CRLF, LFCR */
>> + buf[strcspn(buf, "\r\n")] = '\0';
>
> rtrim_whitespace()?
Ack.
>
>> + strv_push(&stdin_lines, strdup(buf));
>> + }
>> +
>> + argc = strv_length(stdin_lines);
>> + argv = stdin_lines;
>> + }
>> +
>> + CPU_ZERO(&bits);
>> +
>> + for (; argc > 0; argc--, argv++)
>> + if (parse_mask_or_list(*argv, &bits) < 0) {
>> + fprintf(stderr, _("error: invalid argument: %s\n"), *argv);
>> + errtryhelp(EXIT_FAILURE);
>> + }
>> +
>> + strv_free(stdin_lines);
>
> No need to free this here. The process will exit anyways.
I wanted to make ASAN happy. Is this not necessary?
>
>> +
>> + print_bits(&bits, mode);
>> +
>> + return EXIT_SUCCESS;
>> +}
Thanks for the review!
^ permalink raw reply
* Re: [RFC PATCH util-linux] text-utils: add bits command
From: Thomas Weißschuh @ 2024-10-16 21:00 UTC (permalink / raw)
To: Robin Jarry; +Cc: util-linux
In-Reply-To: <20241016202621.2124554-2-robin@jarry.cc>
On 2024-10-16 22:26:22+0200, Robin Jarry wrote:
> Add a new test utility to convert between bit masks in various formats.
"test" or "text" utility?
> This can be handy to avoid parsing affinity masks in one's head and/or
> to interact with the kernel in a more human friendly way.
>
> This is a rewrite in C of the bits command from my linux-tools python
> package so that it can be more widely available.
>
> Here is an example:
>
> ~# cat /sys/kernel/debug/tracing/tracing_cpumask
> fffffff,ffffffff,ffffffff,ffffffff
> ~# bits -l ,$(cat /sys/kernel/debug/tracing/tracing_cpumask)
> 0-128
> ~# bits -g 58,59,120,123
> 9000000,00000000,0c000000,00000000
> ~# bits -g 58,59 > /sys/kernel/debug/tracing/tracing_cpumask
> ~# echo 1 > /sys/kernel/debug/tracing/tracing_on
>
> Add man page and basic tests.
>
> Link: https://git.sr.ht/~rjarry/linux-tools#bits
> Signed-off-by: Robin Jarry <robin@jarry.cc>
> ---
> bash-completion/bits | 21 ++
> meson.build | 11 ++
> tests/commands.sh | 1 +
> tests/expected/misc/bits | 0
> tests/expected/misc/bits-and | 1 +
> tests/expected/misc/bits-binary | 1 +
> tests/expected/misc/bits-default | 1 +
> tests/expected/misc/bits-grouped-mask | 1 +
> tests/expected/misc/bits-list | 1 +
> tests/expected/misc/bits-mask | 1 +
> tests/expected/misc/bits-not | 1 +
> tests/expected/misc/bits-or | 1 +
> tests/expected/misc/bits-overflow | 1 +
> tests/expected/misc/bits-parse-mask | 1 +
> tests/expected/misc/bits-parse-range | 1 +
> tests/expected/misc/bits-stdin | 1 +
> tests/expected/misc/bits-xor | 1 +
> tests/ts/misc/bits | 82 ++++++++
> text-utils/Makemodule.am | 6 +
> text-utils/bits.1.adoc | 147 ++++++++++++++
> text-utils/bits.c | 265 ++++++++++++++++++++++++++
> text-utils/meson.build | 4 +
> 22 files changed, 550 insertions(+)
> create mode 100644 bash-completion/bits
> create mode 100644 tests/expected/misc/bits
> create mode 100644 tests/expected/misc/bits-and
> create mode 100644 tests/expected/misc/bits-binary
> create mode 100644 tests/expected/misc/bits-default
> create mode 100644 tests/expected/misc/bits-grouped-mask
> create mode 100644 tests/expected/misc/bits-list
> create mode 100644 tests/expected/misc/bits-mask
> create mode 100644 tests/expected/misc/bits-not
> create mode 100644 tests/expected/misc/bits-or
> create mode 100644 tests/expected/misc/bits-overflow
> create mode 100644 tests/expected/misc/bits-parse-mask
> create mode 100644 tests/expected/misc/bits-parse-range
> create mode 100644 tests/expected/misc/bits-stdin
> create mode 100644 tests/expected/misc/bits-xor
> create mode 100755 tests/ts/misc/bits
> create mode 100644 text-utils/bits.1.adoc
> create mode 100644 text-utils/bits.c
...
> +++ b/tests/ts/misc/bits
> @@ -0,0 +1,82 @@
> +#!/bin/bash
> +
> +#
> +# Copyright (c) 2024 Robin Jarry
> +#
> +# This file is part of util-linux.
> +#
> +# This file is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This file is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
New files should use SPDX tags.
> diff --git a/text-utils/bits.c b/text-utils/bits.c
> new file mode 100644
> index 000000000000..6dd0db81a5de
> --- /dev/null
> +++ b/text-utils/bits.c
> @@ -0,0 +1,265 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Copyright (c) 2024 Robin Jarry
> + *
> + * bits - convert bit masks from/to various formats
> + */
> +
> +#include <errno.h>
> +#include <getopt.h>
> +#include <sched.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include "c.h"
> +#include "closestream.h"
> +#include "cpuset.h"
> +#include "nls.h"
> +#include "strutils.h"
> +#include "strv.h"
> +
> +typedef enum {
> + OP_OR,
> + OP_AND,
> + OP_NOT,
> + OP_XOR,
> +} bitvise_op_t;
"bitwise"?
Also avoid using the _t suffix as it's reserved.
"enum bitwise_op" seems clear enough.
> +
> +static int parse_mask_or_list(const char *cmdline_arg, cpu_set_t *all_bits)
> +{
> + bitvise_op_t op = OP_OR;
> + cpu_set_t bits, copy;
This seems very geared towards cpu masks.
What happens if a user wants to use it with very large non-CPU bitmasks,
as the tool is advertised as a generic utility?
Call the utility "cpumask"?
> + char buf[BUFSIZ];
Only use BUFSIZ in cases where the actual value does not matter,
for example loops.
On musl BUFSIZ == 1024 and this could not be enough fairly soon.
> + char *arg = buf;
> +
> + /* copy to allow modifying the argument contents */
> + strlcpy(buf, cmdline_arg, sizeof(buf));
> + buf[sizeof(buf) - 1] = '\0';
You can just use xstrdup().
> +
> + /* strip optional operator first */
> + if (startswith(arg, "&")) {
> + op = OP_AND;
> + arg++;
> + } else if (startswith(arg, "^")) {
> + op = OP_XOR;
> + arg++;
> + } else if (startswith(arg, "~")) {
> + op = OP_NOT;
> + arg++;
> + } else if (startswith(arg, "|")) {
> + op = OP_OR;
> + arg++;
> + }
The whole op enum could also be replaced by the actual character,
this is all in a single function.
op = '&';
...
switch (op) {
case '&':
}
> +
> + if (startswith(arg, ",") || startswith(arg, "0x")) {
> + if (cpumask_parse(arg, &bits, sizeof(bits)) < 0)
> + return -EINVAL;
As this is a commandline tool you can call errx() here to kill the
process and provide better error information.
It would also make the call sequence slightly more simple.
> + } else {
> + if (cpulist_parse(arg, &bits, sizeof(bits), 1) < 0)
> + return -EINVAL;
> + }
> +
> + switch (op) {
> + case OP_AND:
> + copy = *all_bits;
> + CPU_AND(all_bits, ©, &bits);
> + break;
> + case OP_OR:
> + copy = *all_bits;
> + CPU_OR(all_bits, ©, &bits);
> + break;
> + case OP_XOR:
> + copy = *all_bits;
> + CPU_XOR(all_bits, ©, &bits);
> + break;
> + case OP_NOT:
> + for (int i = 0; i < CPU_SETSIZE; i++) {
> + if (CPU_ISSET(i, &bits))
> + CPU_CLR(i, all_bits);
> + }
> + break;
> + }
> +
> + return 0;
> +}
> +
> +typedef enum {
> + MODE_BINARY,
> + MODE_GROUPED_MASK,
> + MODE_LIST,
> + MODE_MASK,
> +} output_mode_t;
> +
> +static void print_bits(cpu_set_t *bits, output_mode_t mode)
> +{
> + bool started = false;
> + char buf[BUFSIZ];
> + ssize_t n = 0;
> +
> + buf[0] = '\0';
Is this necessary?
> +
> + switch (mode) {
> + case MODE_MASK:
> + cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
> +
> + /* strip leading zeroes */
> + while (buf[n] == '0')
> + n++;
> + if (buf[n] == '\0')
> + printf("0x0\n");
> + else
> + printf("0x%s\n", buf + n);
> + break;
> +
> + case MODE_GROUPED_MASK:
> + cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
> +
> + /* strip leading zeroes */
> + while (buf[n] == '0')
> + n++;
> +
> + while (buf[n] != '\0') {
> + if (started && (n % 8) == 0)
> + printf(",");
> + if (buf[n] != '0')
> + started = true;
> + printf("%c", buf[n]);
> + n++;
> + }
> + printf("\n");
> + break;
> +
> + case MODE_BINARY:
> + printf("0b");
> + for (n = CPU_SETSIZE; n >= 0; n--) {
> + if (started && ((n + 1) % 4) == 0)
> + printf("_");
> + if (CPU_ISSET(n, bits)) {
> + started = true;
> + printf("1");
> + } else if (started) {
> + printf("0");
> + }
> + }
> + printf("\n");
> + break;
> +
> + case MODE_LIST:
> + cpulist_create(buf, sizeof(buf), bits, sizeof(*bits));
This could use the return values of cpulist_create() and
cpumask_create().
> + printf("%s\n", buf);
> + break;
> + }
> +
> +}
> +
> +static void __attribute__((__noreturn__)) usage(void)
> +{
> + fputs(USAGE_HEADER, stdout);
> + fprintf(stdout, _(" %s [options] [<mask_or_list>...]\n"), program_invocation_short_name);
> +
> + fputs(USAGE_SEPARATOR, stdout);
> + fputsln(_("Convert bit masks from/to various formats."), stdout);
> +
> + fputs(USAGE_ARGUMENTS, stdout);
> + fputsln(_(" <mask_or_list> A set of bits specified as a hex mask value (e.g. 0xeec2)\n"
> + " or as a comma-separated list of bit IDs.\n"
> + " If not specified, arguments will be read from stdin."), stdout);
> +
> + fputs(USAGE_OPTIONS, stdout);
> + fprintf(stdout, USAGE_HELP_OPTIONS(21));
> +
> + fputs(_("\nMode:\n"), stdout);
> + fputsln(_(" -m, --mask Print the combined args as a hex mask value (default).\n"), stdout);
> + fputsln(_(" -g, --grouped-mask Print the combined args as a hex mask value in 32bit\n"
> + " comma separated groups."), stdout);
> + fputsln(_(" -b, --binary Print the combined args as a binary mask value."), stdout);
> + fputsln(_(" -l, --list Print the combined args as a compressed list of bit IDs."), stdout);
> +
> + fputs(USAGE_SEPARATOR, stdout);
> + fprintf(stdout, USAGE_MAN_TAIL("bits(1)"));
> + exit(EXIT_SUCCESS);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + output_mode_t mode = MODE_MASK;
> + char **stdin_lines = NULL;
> + cpu_set_t bits;
> + int c;
> +
> + static const struct option longopts[] = {
> + { "version", no_argument, NULL, 'V'},
> + { "help", no_argument, NULL, 'h'},
> + { "mask", no_argument, NULL, 'm'},
> + { "grouped-mask", no_argument, NULL, 'g'},
> + { "binary", no_argument, NULL, 'b'},
> + { "list", no_argument, NULL, 'l'},
> + { NULL, 0, NULL, 0 }
> + };
> +
> + setlocale(LC_ALL, "");
> + bindtextdomain(PACKAGE, LOCALEDIR);
> + textdomain(PACKAGE);
> + close_stdout_atexit();
> +
> + while ((c = getopt_long(argc, argv, "Vhmgbl", longopts, NULL)) != -1)
> + switch (c) {
> + case 'm':
> + mode = MODE_MASK;
> + break;
> + case 'g':
> + mode = MODE_GROUPED_MASK;
> + break;
> + case 'b':
> + mode = MODE_BINARY;
> + break;
> + case 'l':
> + mode = MODE_LIST;
> + break;
> + case 'V':
> + print_version(EXIT_SUCCESS);
> + case 'h':
> + usage();
> + default:
> + errtryhelp(EXIT_FAILURE);
> + }
> +
> + argc -= optind;
> + argv += optind;
> + if (argc == 0) {
> + /* no arguments provided, read lines from stdin */
> + char buf[BUFSIZ];
> +
> + while (fgets(buf, sizeof(buf), stdin)) {
> + /* strip LF, CR, CRLF, LFCR */
> + buf[strcspn(buf, "\r\n")] = '\0';
rtrim_whitespace()?
> + strv_push(&stdin_lines, strdup(buf));
> + }
> +
> + argc = strv_length(stdin_lines);
> + argv = stdin_lines;
> + }
> +
> + CPU_ZERO(&bits);
> +
> + for (; argc > 0; argc--, argv++)
> + if (parse_mask_or_list(*argv, &bits) < 0) {
> + fprintf(stderr, _("error: invalid argument: %s\n"), *argv);
> + errtryhelp(EXIT_FAILURE);
> + }
> +
> + strv_free(stdin_lines);
No need to free this here. The process will exit anyways.
> +
> + print_bits(&bits, mode);
> +
> + return EXIT_SUCCESS;
> +}
^ permalink raw reply
* [RFC PATCH util-linux] text-utils: add bits command
From: Robin Jarry @ 2024-10-16 20:26 UTC (permalink / raw)
To: util-linux
Add a new test utility to convert between bit masks in various formats.
This can be handy to avoid parsing affinity masks in one's head and/or
to interact with the kernel in a more human friendly way.
This is a rewrite in C of the bits command from my linux-tools python
package so that it can be more widely available.
Here is an example:
~# cat /sys/kernel/debug/tracing/tracing_cpumask
fffffff,ffffffff,ffffffff,ffffffff
~# bits -l ,$(cat /sys/kernel/debug/tracing/tracing_cpumask)
0-128
~# bits -g 58,59,120,123
9000000,00000000,0c000000,00000000
~# bits -g 58,59 > /sys/kernel/debug/tracing/tracing_cpumask
~# echo 1 > /sys/kernel/debug/tracing/tracing_on
Add man page and basic tests.
Link: https://git.sr.ht/~rjarry/linux-tools#bits
Signed-off-by: Robin Jarry <robin@jarry.cc>
---
bash-completion/bits | 21 ++
meson.build | 11 ++
tests/commands.sh | 1 +
tests/expected/misc/bits | 0
tests/expected/misc/bits-and | 1 +
tests/expected/misc/bits-binary | 1 +
tests/expected/misc/bits-default | 1 +
tests/expected/misc/bits-grouped-mask | 1 +
tests/expected/misc/bits-list | 1 +
tests/expected/misc/bits-mask | 1 +
tests/expected/misc/bits-not | 1 +
tests/expected/misc/bits-or | 1 +
tests/expected/misc/bits-overflow | 1 +
tests/expected/misc/bits-parse-mask | 1 +
tests/expected/misc/bits-parse-range | 1 +
tests/expected/misc/bits-stdin | 1 +
tests/expected/misc/bits-xor | 1 +
tests/ts/misc/bits | 82 ++++++++
text-utils/Makemodule.am | 6 +
text-utils/bits.1.adoc | 147 ++++++++++++++
text-utils/bits.c | 265 ++++++++++++++++++++++++++
text-utils/meson.build | 4 +
22 files changed, 550 insertions(+)
create mode 100644 bash-completion/bits
create mode 100644 tests/expected/misc/bits
create mode 100644 tests/expected/misc/bits-and
create mode 100644 tests/expected/misc/bits-binary
create mode 100644 tests/expected/misc/bits-default
create mode 100644 tests/expected/misc/bits-grouped-mask
create mode 100644 tests/expected/misc/bits-list
create mode 100644 tests/expected/misc/bits-mask
create mode 100644 tests/expected/misc/bits-not
create mode 100644 tests/expected/misc/bits-or
create mode 100644 tests/expected/misc/bits-overflow
create mode 100644 tests/expected/misc/bits-parse-mask
create mode 100644 tests/expected/misc/bits-parse-range
create mode 100644 tests/expected/misc/bits-stdin
create mode 100644 tests/expected/misc/bits-xor
create mode 100755 tests/ts/misc/bits
create mode 100644 text-utils/bits.1.adoc
create mode 100644 text-utils/bits.c
diff --git a/bash-completion/bits b/bash-completion/bits
new file mode 100644
index 000000000000..763e8669cfb2
--- /dev/null
+++ b/bash-completion/bits
@@ -0,0 +1,21 @@
+_bits_module()
+{
+ local cur prev OPTS
+ COMPREPLY=()
+ cur="${COMP_WORDS[COMP_CWORD]}"
+ prev="${COMP_WORDS[COMP_CWORD-1]}"
+ case $prev in
+ '-h'|'--help'|'-V'|'--version')
+ return 0
+ ;;
+ esac
+ case $cur in
+ -*)
+ OPTS="--version --help --mask --grouped-mask --bit --list"
+ COMPREPLY=( $(compgen -W "${OPTS[*]}" -- $cur) )
+ return 0
+ ;;
+ esac
+ return 0
+}
+complete -F _bits_module bits
diff --git a/meson.build b/meson.build
index a78ebd6b8934..db1c28f42bd3 100644
--- a/meson.build
+++ b/meson.build
@@ -1201,6 +1201,17 @@ endif
############################################################
+exe = executable(
+ 'bits',
+ bits_sources,
+ include_directories : includes,
+ link_with : lib_common,
+ install_dir : usrbin_exec_dir,
+ install : true)
+exes += exe
+manadocs += ['text-utils/bits.1.adoc']
+bashcompletions += ['bits']
+
if is_glibc
exe = executable(
'col',
diff --git a/tests/commands.sh b/tests/commands.sh
index 9eef92ccbb72..4402e38b4118 100644
--- a/tests/commands.sh
+++ b/tests/commands.sh
@@ -64,6 +64,7 @@ TS_HELPER_TIMEUTILS="${ts_helpersdir}test_timeutils"
# paths to commands
TS_CMD_ADDPART=${TS_CMD_ADDPART:-"${ts_commandsdir}addpart"}
TS_CMD_DELPART=${TS_CMD_DELPART:-"${ts_commandsdir}delpart"}
+TS_CMD_BITS=${TS_CMD_BITS-"${ts_commandsdir}bits"}
TS_CMD_BLKDISCARD=${TS_CMD_BLKID-"${ts_commandsdir}blkdiscard"}
TS_CMD_BLKID=${TS_CMD_BLKID-"${ts_commandsdir}blkid"}
TS_CMD_CAL=${TS_CMD_CAL-"${ts_commandsdir}cal"}
diff --git a/tests/expected/misc/bits b/tests/expected/misc/bits
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/expected/misc/bits-and b/tests/expected/misc/bits-and
new file mode 100644
index 000000000000..1d8ffee8c97a
--- /dev/null
+++ b/tests/expected/misc/bits-and
@@ -0,0 +1 @@
+75-100
diff --git a/tests/expected/misc/bits-binary b/tests/expected/misc/bits-binary
new file mode 100644
index 000000000000..ba7b220e9354
--- /dev/null
+++ b/tests/expected/misc/bits-binary
@@ -0,0 +1 @@
+0b1_0000_0000_0010_0000_0000_0100_0000_0000_1000_0000_0000
diff --git a/tests/expected/misc/bits-default b/tests/expected/misc/bits-default
new file mode 100644
index 000000000000..a56502d4c880
--- /dev/null
+++ b/tests/expected/misc/bits-default
@@ -0,0 +1 @@
+0x100200400800
diff --git a/tests/expected/misc/bits-grouped-mask b/tests/expected/misc/bits-grouped-mask
new file mode 100644
index 000000000000..427fc5c2a6ca
--- /dev/null
+++ b/tests/expected/misc/bits-grouped-mask
@@ -0,0 +1 @@
+1002,00400800
diff --git a/tests/expected/misc/bits-list b/tests/expected/misc/bits-list
new file mode 100644
index 000000000000..7511e5378ea4
--- /dev/null
+++ b/tests/expected/misc/bits-list
@@ -0,0 +1 @@
+11,22,33,44
diff --git a/tests/expected/misc/bits-mask b/tests/expected/misc/bits-mask
new file mode 100644
index 000000000000..a56502d4c880
--- /dev/null
+++ b/tests/expected/misc/bits-mask
@@ -0,0 +1 @@
+0x100200400800
diff --git a/tests/expected/misc/bits-not b/tests/expected/misc/bits-not
new file mode 100644
index 000000000000..2487fcfecb22
--- /dev/null
+++ b/tests/expected/misc/bits-not
@@ -0,0 +1 @@
+50-74
diff --git a/tests/expected/misc/bits-or b/tests/expected/misc/bits-or
new file mode 100644
index 000000000000..753370ac5c3b
--- /dev/null
+++ b/tests/expected/misc/bits-or
@@ -0,0 +1 @@
+50-150
diff --git a/tests/expected/misc/bits-overflow b/tests/expected/misc/bits-overflow
new file mode 100644
index 000000000000..9982566dc094
--- /dev/null
+++ b/tests/expected/misc/bits-overflow
@@ -0,0 +1 @@
+0x0
diff --git a/tests/expected/misc/bits-parse-mask b/tests/expected/misc/bits-parse-mask
new file mode 100644
index 000000000000..59dd4b4c1696
--- /dev/null
+++ b/tests/expected/misc/bits-parse-mask
@@ -0,0 +1 @@
+1,3,6,7,9,11,14-16,18,19,21,23-25,27
diff --git a/tests/expected/misc/bits-parse-range b/tests/expected/misc/bits-parse-range
new file mode 100644
index 000000000000..5afeb0332629
--- /dev/null
+++ b/tests/expected/misc/bits-parse-range
@@ -0,0 +1 @@
+7fffff,ffffffff,ffffffff,fffc0000,00000000
diff --git a/tests/expected/misc/bits-stdin b/tests/expected/misc/bits-stdin
new file mode 100644
index 000000000000..00ff99b2a15c
--- /dev/null
+++ b/tests/expected/misc/bits-stdin
@@ -0,0 +1 @@
+11,33,44
diff --git a/tests/expected/misc/bits-xor b/tests/expected/misc/bits-xor
new file mode 100644
index 000000000000..55289438dc57
--- /dev/null
+++ b/tests/expected/misc/bits-xor
@@ -0,0 +1 @@
+50-74,101-150
diff --git a/tests/ts/misc/bits b/tests/ts/misc/bits
new file mode 100755
index 000000000000..e76677dd56b0
--- /dev/null
+++ b/tests/ts/misc/bits
@@ -0,0 +1,82 @@
+#!/bin/bash
+
+#
+# Copyright (c) 2024 Robin Jarry
+#
+# This file is part of util-linux.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This file is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="bits"
+
+. "$TS_TOPDIR"/functions.sh
+ts_init "$*"
+
+ts_check_test_command "$TS_CMD_BITS"
+ts_cd "$TS_OUTDIR"
+
+ts_init_subtest "default"
+$TS_CMD_BITS 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "mask"
+$TS_CMD_BITS --mask 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "grouped-mask"
+$TS_CMD_BITS --grouped-mask 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "list"
+$TS_CMD_BITS --list 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "binary"
+$TS_CMD_BITS --binary 11,22,33,44 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "overflow"
+$TS_CMD_BITS 129837984734 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "parse-mask"
+$TS_CMD_BITS -l 0x0badcaca >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "parse-range"
+$TS_CMD_BITS -g 50-100 75-150 >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "or"
+$TS_CMD_BITS -l 50-100 '|75-150' >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "and"
+$TS_CMD_BITS -l 50-100 '&75-150' >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "xor"
+$TS_CMD_BITS -l 50-100 '^75-150' >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "not"
+$TS_CMD_BITS -l 50-100 '~75-150' >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_init_subtest "stdin"
+{
+ echo 11,22,33,44
+ echo ^22
+} | $TS_CMD_BITS --list >> $TS_OUTPUT 2>> $TS_ERRLOG
+ts_finalize_subtest
+
+ts_finalize
diff --git a/text-utils/Makemodule.am b/text-utils/Makemodule.am
index af1bf0238664..28aa7963561d 100644
--- a/text-utils/Makemodule.am
+++ b/text-utils/Makemodule.am
@@ -106,3 +106,9 @@ test_more_LDADD = $(more_LDADD)
endif # BUILD_MORE
+if BUILD_BITS
+usrbin_exec_PROGRAMS += bits
+MANPAGES += text-utils/bits.1
+dist_noinst_DATA += text-utils/bits.1.adoc
+bits_SOURCES = text-utils/bits.c
+endif
diff --git a/text-utils/bits.1.adoc b/text-utils/bits.1.adoc
new file mode 100644
index 000000000000..1cbe3f95ff37
--- /dev/null
+++ b/text-utils/bits.1.adoc
@@ -0,0 +1,147 @@
+//po4a: entry man manual
+////
+Copyright (c) 2024 Robin Jarry
+
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+1. Redistributions of source code must retain the above copyright
+ notice, this list of conditions and the following disclaimer.
+2. Redistributions in binary form must reproduce the above copyright
+ notice, this list of conditions and the following disclaimer in the
+ documentation and/or other materials provided with the distribution.
+3. All advertising materials mentioning features or use of this software
+ must display the following acknowledgement:
+ This product includes software developed by the University of
+ California, Berkeley and its contributors.
+4. Neither the name of the University nor the names of its contributors
+ may be used to endorse or promote products derived from this software
+ without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+SUCH DAMAGE.
+////
+= bits(1)
+:doctype: manpage
+:man manual: User Commands
+:man source: util-linux {release-version}
+:page-layout: base
+:command: bits
+
+== NAME
+
+bits - convert bit masks from/to various formats
+
+== SYNOPSIS
+
+*bits* [*-h*] [*-V*] [_<MODE>_] [_<MASK_OR_LIST>_...]
+
+== DESCRIPTION
+
+The *bits* utility converts bit masks into various formats. It supports
+combining multiple masks together using bitwise operations.
+
+== POSITIONAL ARGUMENTS
+
+_<MASK_OR_LIST>_::
+A set of bits specified as a hexadecimal mask value (e.g. _0xeec2_) or as
+a comma-separated list of bit IDs.
+
+If no argument is specified, the sets of bits will be read from standard input;
+one group per line.
+
+Consecutive ids can be compressed as ranges (e.g. _5,6,7,8,9,10_ -> _5-10_).
+
+Optionally, if an argument starts with a comma, it will be parsed as a single
+hexadecimal mask split in 32bit groups (e.g. _,00014000,00000000,00020000_ ->
+_17,78,80_).
+
+By default all groups will be OR'ed together. If a group has one of the
+following prefixes, it will be combined with the resulting mask using
+a different binary operation:
+
+**&**__<MASK_OR_LIST>__::
+The group will be combined with a binary AND operation. I.e. all bits that are
+set to 1 in the group AND the combined groups so far will be preserved to 1.
+All other bits will be reset to 0.
+
+**^**__<MASK_OR_LIST>__::
+The group will be combined with a binary XOR operation. I.e. all bits that are
+set to 1 in the group AND to 0 the combined groups so far (or the other way
+around) will be set to 1. Bits that are both to 1 or both to 0 will be reset to
+0.
+
+**~**__<MASK_OR_LIST>__::
+All bits set to 1 in the group will be cleared (reset to 0) in the combined
+groups so far.
+
+== OPTIONS
+
+include::man-common/help-version.adoc[]
+
+== CONVERSION MODE
+
+One of the following conversion modes can be specified. If not specified, it
+defaults to *-m*, *--mask*.
+
+*-m*, *--mask*::
+Print the combined args as a hexadecimal mask value (default).
+
+*-g*, *--grouped-mask*::
+Print the combined args as a hexadecimal mask value in 32bit comma separated
+groups.
+
+*-b*, *--binary*::
+Print the combined args as a binary mask value.
+
+*-l*, *--list*::
+Print the combined args as a list of bit IDs. Consecutive IDs are compressed as
+ranges.
+
+== EXAMPLES
+
+....
+~$ bits --mask 4,5-8 16,30
+0x400101f0
+
+~$ bits --list 0xeec2
+1,6,7,9-11,13-15
+
+~$ bits --binary 4,5-8 16,30
+0b100_0000_0000_0001_0000_0001_1111_0000
+
+~$ bits --list ,00300000,03000000,30000003
+0,1,28,29,56,57,84,85
+
+~$ bits --list 1,2,3,4 ~3-10
+1,2
+
+~$ bits --list 1,2,3,4 ^3-10
+1,2,5-10
+
+~$ bits --grouped-mask 2,22,74,79
+8400,00000000,00400004
+....
+
+== AUTHORS
+
+Robin Jarry.
+
+include::man-common/bugreports.adoc[]
+
+include::man-common/footer.adoc[]
+
+ifdef::translation[]
+include::man-common/translation.adoc[]
+endif::[]
diff --git a/text-utils/bits.c b/text-utils/bits.c
new file mode 100644
index 000000000000..6dd0db81a5de
--- /dev/null
+++ b/text-utils/bits.c
@@ -0,0 +1,265 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Copyright (c) 2024 Robin Jarry
+ *
+ * bits - convert bit masks from/to various formats
+ */
+
+#include <errno.h>
+#include <getopt.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "c.h"
+#include "closestream.h"
+#include "cpuset.h"
+#include "nls.h"
+#include "strutils.h"
+#include "strv.h"
+
+typedef enum {
+ OP_OR,
+ OP_AND,
+ OP_NOT,
+ OP_XOR,
+} bitvise_op_t;
+
+static int parse_mask_or_list(const char *cmdline_arg, cpu_set_t *all_bits)
+{
+ bitvise_op_t op = OP_OR;
+ cpu_set_t bits, copy;
+ char buf[BUFSIZ];
+ char *arg = buf;
+
+ /* copy to allow modifying the argument contents */
+ strlcpy(buf, cmdline_arg, sizeof(buf));
+ buf[sizeof(buf) - 1] = '\0';
+
+ /* strip optional operator first */
+ if (startswith(arg, "&")) {
+ op = OP_AND;
+ arg++;
+ } else if (startswith(arg, "^")) {
+ op = OP_XOR;
+ arg++;
+ } else if (startswith(arg, "~")) {
+ op = OP_NOT;
+ arg++;
+ } else if (startswith(arg, "|")) {
+ op = OP_OR;
+ arg++;
+ }
+
+ if (startswith(arg, ",") || startswith(arg, "0x")) {
+ if (cpumask_parse(arg, &bits, sizeof(bits)) < 0)
+ return -EINVAL;
+ } else {
+ if (cpulist_parse(arg, &bits, sizeof(bits), 1) < 0)
+ return -EINVAL;
+ }
+
+ switch (op) {
+ case OP_AND:
+ copy = *all_bits;
+ CPU_AND(all_bits, ©, &bits);
+ break;
+ case OP_OR:
+ copy = *all_bits;
+ CPU_OR(all_bits, ©, &bits);
+ break;
+ case OP_XOR:
+ copy = *all_bits;
+ CPU_XOR(all_bits, ©, &bits);
+ break;
+ case OP_NOT:
+ for (int i = 0; i < CPU_SETSIZE; i++) {
+ if (CPU_ISSET(i, &bits))
+ CPU_CLR(i, all_bits);
+ }
+ break;
+ }
+
+ return 0;
+}
+
+typedef enum {
+ MODE_BINARY,
+ MODE_GROUPED_MASK,
+ MODE_LIST,
+ MODE_MASK,
+} output_mode_t;
+
+static void print_bits(cpu_set_t *bits, output_mode_t mode)
+{
+ bool started = false;
+ char buf[BUFSIZ];
+ ssize_t n = 0;
+
+ buf[0] = '\0';
+
+ switch (mode) {
+ case MODE_MASK:
+ cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
+
+ /* strip leading zeroes */
+ while (buf[n] == '0')
+ n++;
+ if (buf[n] == '\0')
+ printf("0x0\n");
+ else
+ printf("0x%s\n", buf + n);
+ break;
+
+ case MODE_GROUPED_MASK:
+ cpumask_create(buf, sizeof(buf), bits, sizeof(*bits));
+
+ /* strip leading zeroes */
+ while (buf[n] == '0')
+ n++;
+
+ while (buf[n] != '\0') {
+ if (started && (n % 8) == 0)
+ printf(",");
+ if (buf[n] != '0')
+ started = true;
+ printf("%c", buf[n]);
+ n++;
+ }
+ printf("\n");
+ break;
+
+ case MODE_BINARY:
+ printf("0b");
+ for (n = CPU_SETSIZE; n >= 0; n--) {
+ if (started && ((n + 1) % 4) == 0)
+ printf("_");
+ if (CPU_ISSET(n, bits)) {
+ started = true;
+ printf("1");
+ } else if (started) {
+ printf("0");
+ }
+ }
+ printf("\n");
+ break;
+
+ case MODE_LIST:
+ cpulist_create(buf, sizeof(buf), bits, sizeof(*bits));
+ printf("%s\n", buf);
+ break;
+ }
+
+}
+
+static void __attribute__((__noreturn__)) usage(void)
+{
+ fputs(USAGE_HEADER, stdout);
+ fprintf(stdout, _(" %s [options] [<mask_or_list>...]\n"), program_invocation_short_name);
+
+ fputs(USAGE_SEPARATOR, stdout);
+ fputsln(_("Convert bit masks from/to various formats."), stdout);
+
+ fputs(USAGE_ARGUMENTS, stdout);
+ fputsln(_(" <mask_or_list> A set of bits specified as a hex mask value (e.g. 0xeec2)\n"
+ " or as a comma-separated list of bit IDs.\n"
+ " If not specified, arguments will be read from stdin."), stdout);
+
+ fputs(USAGE_OPTIONS, stdout);
+ fprintf(stdout, USAGE_HELP_OPTIONS(21));
+
+ fputs(_("\nMode:\n"), stdout);
+ fputsln(_(" -m, --mask Print the combined args as a hex mask value (default).\n"), stdout);
+ fputsln(_(" -g, --grouped-mask Print the combined args as a hex mask value in 32bit\n"
+ " comma separated groups."), stdout);
+ fputsln(_(" -b, --binary Print the combined args as a binary mask value."), stdout);
+ fputsln(_(" -l, --list Print the combined args as a compressed list of bit IDs."), stdout);
+
+ fputs(USAGE_SEPARATOR, stdout);
+ fprintf(stdout, USAGE_MAN_TAIL("bits(1)"));
+ exit(EXIT_SUCCESS);
+}
+
+int main(int argc, char **argv)
+{
+ output_mode_t mode = MODE_MASK;
+ char **stdin_lines = NULL;
+ cpu_set_t bits;
+ int c;
+
+ static const struct option longopts[] = {
+ { "version", no_argument, NULL, 'V'},
+ { "help", no_argument, NULL, 'h'},
+ { "mask", no_argument, NULL, 'm'},
+ { "grouped-mask", no_argument, NULL, 'g'},
+ { "binary", no_argument, NULL, 'b'},
+ { "list", no_argument, NULL, 'l'},
+ { NULL, 0, NULL, 0 }
+ };
+
+ setlocale(LC_ALL, "");
+ bindtextdomain(PACKAGE, LOCALEDIR);
+ textdomain(PACKAGE);
+ close_stdout_atexit();
+
+ while ((c = getopt_long(argc, argv, "Vhmgbl", longopts, NULL)) != -1)
+ switch (c) {
+ case 'm':
+ mode = MODE_MASK;
+ break;
+ case 'g':
+ mode = MODE_GROUPED_MASK;
+ break;
+ case 'b':
+ mode = MODE_BINARY;
+ break;
+ case 'l':
+ mode = MODE_LIST;
+ break;
+ case 'V':
+ print_version(EXIT_SUCCESS);
+ case 'h':
+ usage();
+ default:
+ errtryhelp(EXIT_FAILURE);
+ }
+
+ argc -= optind;
+ argv += optind;
+ if (argc == 0) {
+ /* no arguments provided, read lines from stdin */
+ char buf[BUFSIZ];
+
+ while (fgets(buf, sizeof(buf), stdin)) {
+ /* strip LF, CR, CRLF, LFCR */
+ buf[strcspn(buf, "\r\n")] = '\0';
+ strv_push(&stdin_lines, strdup(buf));
+ }
+
+ argc = strv_length(stdin_lines);
+ argv = stdin_lines;
+ }
+
+ CPU_ZERO(&bits);
+
+ for (; argc > 0; argc--, argv++)
+ if (parse_mask_or_list(*argv, &bits) < 0) {
+ fprintf(stderr, _("error: invalid argument: %s\n"), *argv);
+ errtryhelp(EXIT_FAILURE);
+ }
+
+ strv_free(stdin_lines);
+
+ print_bits(&bits, mode);
+
+ return EXIT_SUCCESS;
+}
diff --git a/text-utils/meson.build b/text-utils/meson.build
index f3b25d382160..4a25fa478328 100644
--- a/text-utils/meson.build
+++ b/text-utils/meson.build
@@ -1,3 +1,7 @@
+bits_sources = files(
+ 'bits.c',
+)
+
col_sources = files(
'col.c',
)
--
2.47.0
^ permalink raw reply related
* Re: Libmount bug ?
From: Alan Huang @ 2024-10-15 11:44 UTC (permalink / raw)
To: Karel Zak
Cc: util-linux, linux-bcachefs, Kent Overstreet, Hongbo Li,
Darrick J. Wong, xfs
In-Reply-To: <iipuwlnf73x3zjj4kgpgqqvu4u5t4iefg3qawqgzvl546rrbz5@w7tvj3jr5h2v>
On Oct 14, 2024, at 23:57, Karel Zak <kzak@redhat.com> wrote:
>
>
> Hi Alan,
>
> On Sat, Oct 12, 2024 at 04:57:39PM GMT, Alan Huang wrote:
>> The bcachefs has the helper called mount.bcachefs.
>
> do you mean the following script?
> https://evilpiepirate.org/git/bcachefs-tools.git/tree/mount.bcachefs.sh
Not this one, but the Rust code:
https://evilpiepirate.org/git/bcachefs-tools.git/tree/src/commands/mount.rs
>
> I believe that if you call the regular mount(8) from the script, then
> it's probably fine to not worry about the options. mount(8) will be
> able to ignore them.
>
>> Currently, there are users using fstab with nofail/user fail to mount,
>> we would like to know whether other filesystems using similar helper
>> properly handle this.
>
> The mount.nfs command uses libmount internally to generate the
> mount(2) syscall flags, so it is not affected by any additional
> options.
>
> The mount.fuse command has a list of unwanted mount options:
> https://github.com/libfuse/libfuse/blob/master/util/mount.fuse.c#L318-L326
It seems that no other kernel filesystems are using similar helpers.
>
> Please note that the "EXTERNAL HELPERS" section in the mount(8) man
> page describes which options are ignored.
>
> Also, if your mount helper is setuid (like mount.nfs), you still need
> to parse fstab to obtain mount options from a safe source. This is
> because options from the command line should be ignored as they are
> considered unsafe.
Good to know.
>
>>
>> This is like commit 06e05eb0f78566b68c44328c37d7c28d8655e9df
>> (“libmount: don't pass option "defaults" to helper")
>>
>> Or would you like something like this? This might be incomplete though (e.g. owner, noowner etc.)
>>
>> diff --git a/libmount/src/optmap.c b/libmount/src/optmap.c
>> index d7569a0f0..c13b9ba19 100644
>> --- a/libmount/src/optmap.c
>> +++ b/libmount/src/optmap.c
>> @@ -152,11 +152,11 @@ static const struct libmnt_optmap userspace_opts_map[] =
>> { "auto", MNT_MS_NOAUTO, MNT_NOHLPS | MNT_INVERT | MNT_NOMTAB }, /* Can be mounted using -a */
>> { "noauto", MNT_MS_NOAUTO, MNT_NOHLPS | MNT_NOMTAB }, /* Can only be mounted explicitly */
>>
>> - { "user[=]", MNT_MS_USER }, /* Allow ordinary user to mount (mtab) */
>> - { "nouser", MNT_MS_USER, MNT_INVERT | MNT_NOMTAB }, /* Forbid ordinary user to mount */
>> + { "user[=]", MNT_MS_USER, MNT_NOHLPS}, /* Allow ordinary user to mount (mtab) */
>
> This may cause issues with certain helpers (e.g. cifs) where "user="
> is a standard option. However, this is something that needs to be
> addressed in libmount, as it already handles this use-case for cifs.
> The use of MNT_NOHLPS may override this.
Yeah, I was worried that there might be helpers using these options.
>
>> + { "nouser", MNT_MS_USER, MNT_INVERT | MNT_NOMTAB | MNT_NOHLPS}, /* Forbid ordinary user to mount */
>>
>> - { "users", MNT_MS_USERS, MNT_NOMTAB }, /* Allow ordinary users to mount */
>> - { "nousers", MNT_MS_USERS, MNT_INVERT | MNT_NOMTAB }, /* Forbid ordinary users to mount */
>> + { "users", MNT_MS_USERS, MNT_NOMTAB | MNT_NOHLPS}, /* Allow ordinary users to mount */
>> + { "nousers", MNT_MS_USERS, MNT_INVERT | MNT_NOMTAB | MNT_NOHLPS}, /* Forbid ordinary users to mount */
>>
>> { "owner", MNT_MS_OWNER, MNT_NOMTAB }, /* Let the owner of the device mount */
>> { "noowner", MNT_MS_OWNER, MNT_INVERT | MNT_NOMTAB }, /* Device owner has no special privs */
>> @@ -180,7 +180,7 @@ static const struct libmnt_optmap userspace_opts_map[] =
>> { "sizelimit=", MNT_MS_SIZELIMIT, MNT_NOHLPS | MNT_NOMTAB }, /* loop device size limit */
>> { "encryption=", MNT_MS_ENCRYPTION, MNT_NOHLPS | MNT_NOMTAB }, /* loop device encryption */
>>
>> - { "nofail", MNT_MS_NOFAIL, MNT_NOMTAB }, /* Do not fail if ENOENT on dev */
>> + { "nofail", MNT_MS_NOFAIL, MNT_NOMTAB | MNT_NOHLPS}, /* Do not fail if ENOENT on dev */
>
> Could this option be usable for some helpers?
>
> I believe the best solution is to follow the Fuse way and define a
> list of options to ignore in your fs-specific helper.
>
> The ideal solution would be to implement a better libmount (perhaps
> libmount2) where the /sbin/mount.<type> helpers are replaced with
> dlopen() modules. This way, the library would handle all the details
> such as command line and fstab options.
Agreed.
Thanks,
Alan
> Karel
>
> --
> Karel Zak <kzak@redhat.com>
> http://karelzak.blogspot.com
>
^ permalink raw reply
* Re: Libmount bug ?
From: Karel Zak @ 2024-10-14 15:57 UTC (permalink / raw)
To: Alan Huang
Cc: util-linux, linux-bcachefs, Kent Overstreet, Hongbo Li,
Darrick J. Wong, xfs
In-Reply-To: <14ADF290-5B46-44D5-83BC-9AE3732B192C@gmail.com>
Hi Alan,
On Sat, Oct 12, 2024 at 04:57:39PM GMT, Alan Huang wrote:
> The bcachefs has the helper called mount.bcachefs.
do you mean the following script?
https://evilpiepirate.org/git/bcachefs-tools.git/tree/mount.bcachefs.sh
I believe that if you call the regular mount(8) from the script, then
it's probably fine to not worry about the options. mount(8) will be
able to ignore them.
> Currently, there are users using fstab with nofail/user fail to mount,
> we would like to know whether other filesystems using similar helper
> properly handle this.
The mount.nfs command uses libmount internally to generate the
mount(2) syscall flags, so it is not affected by any additional
options.
The mount.fuse command has a list of unwanted mount options:
https://github.com/libfuse/libfuse/blob/master/util/mount.fuse.c#L318-L326
Please note that the "EXTERNAL HELPERS" section in the mount(8) man
page describes which options are ignored.
Also, if your mount helper is setuid (like mount.nfs), you still need
to parse fstab to obtain mount options from a safe source. This is
because options from the command line should be ignored as they are
considered unsafe.
>
> This is like commit 06e05eb0f78566b68c44328c37d7c28d8655e9df
> (“libmount: don't pass option "defaults" to helper")
>
> Or would you like something like this? This might be incomplete though (e.g. owner, noowner etc.)
>
> diff --git a/libmount/src/optmap.c b/libmount/src/optmap.c
> index d7569a0f0..c13b9ba19 100644
> --- a/libmount/src/optmap.c
> +++ b/libmount/src/optmap.c
> @@ -152,11 +152,11 @@ static const struct libmnt_optmap userspace_opts_map[] =
> { "auto", MNT_MS_NOAUTO, MNT_NOHLPS | MNT_INVERT | MNT_NOMTAB }, /* Can be mounted using -a */
> { "noauto", MNT_MS_NOAUTO, MNT_NOHLPS | MNT_NOMTAB }, /* Can only be mounted explicitly */
>
> - { "user[=]", MNT_MS_USER }, /* Allow ordinary user to mount (mtab) */
> - { "nouser", MNT_MS_USER, MNT_INVERT | MNT_NOMTAB }, /* Forbid ordinary user to mount */
> + { "user[=]", MNT_MS_USER, MNT_NOHLPS}, /* Allow ordinary user to mount (mtab) */
This may cause issues with certain helpers (e.g. cifs) where "user="
is a standard option. However, this is something that needs to be
addressed in libmount, as it already handles this use-case for cifs.
The use of MNT_NOHLPS may override this.
> + { "nouser", MNT_MS_USER, MNT_INVERT | MNT_NOMTAB | MNT_NOHLPS}, /* Forbid ordinary user to mount */
>
> - { "users", MNT_MS_USERS, MNT_NOMTAB }, /* Allow ordinary users to mount */
> - { "nousers", MNT_MS_USERS, MNT_INVERT | MNT_NOMTAB }, /* Forbid ordinary users to mount */
> + { "users", MNT_MS_USERS, MNT_NOMTAB | MNT_NOHLPS}, /* Allow ordinary users to mount */
> + { "nousers", MNT_MS_USERS, MNT_INVERT | MNT_NOMTAB | MNT_NOHLPS}, /* Forbid ordinary users to mount */
>
> { "owner", MNT_MS_OWNER, MNT_NOMTAB }, /* Let the owner of the device mount */
> { "noowner", MNT_MS_OWNER, MNT_INVERT | MNT_NOMTAB }, /* Device owner has no special privs */
> @@ -180,7 +180,7 @@ static const struct libmnt_optmap userspace_opts_map[] =
> { "sizelimit=", MNT_MS_SIZELIMIT, MNT_NOHLPS | MNT_NOMTAB }, /* loop device size limit */
> { "encryption=", MNT_MS_ENCRYPTION, MNT_NOHLPS | MNT_NOMTAB }, /* loop device encryption */
>
> - { "nofail", MNT_MS_NOFAIL, MNT_NOMTAB }, /* Do not fail if ENOENT on dev */
> + { "nofail", MNT_MS_NOFAIL, MNT_NOMTAB | MNT_NOHLPS}, /* Do not fail if ENOENT on dev */
Could this option be usable for some helpers?
I believe the best solution is to follow the Fuse way and define a
list of options to ignore in your fs-specific helper.
The ideal solution would be to implement a better libmount (perhaps
libmount2) where the /sbin/mount.<type> helpers are replaced with
dlopen() modules. This way, the library would handle all the details
such as command line and fstab options.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Libmount bug ?
From: Alan Huang @ 2024-10-12 8:57 UTC (permalink / raw)
To: kzak
Cc: util-linux, linux-bcachefs, Kent Overstreet, Hongbo Li,
Darrick J. Wong, xfs
Hello Karel,
The bcachefs has the helper called mount.bcachefs.
Currently, there are users using fstab with nofail/user fail to mount,
we would like to know whether other filesystems using similar helper
properly handle this.
This is like commit 06e05eb0f78566b68c44328c37d7c28d8655e9df
(“libmount: don't pass option "defaults" to helper")
Or would you like something like this? This might be incomplete though (e.g. owner, noowner etc.)
diff --git a/libmount/src/optmap.c b/libmount/src/optmap.c
index d7569a0f0..c13b9ba19 100644
--- a/libmount/src/optmap.c
+++ b/libmount/src/optmap.c
@@ -152,11 +152,11 @@ static const struct libmnt_optmap userspace_opts_map[] =
{ "auto", MNT_MS_NOAUTO, MNT_NOHLPS | MNT_INVERT | MNT_NOMTAB }, /* Can be mounted using -a */
{ "noauto", MNT_MS_NOAUTO, MNT_NOHLPS | MNT_NOMTAB }, /* Can only be mounted explicitly */
- { "user[=]", MNT_MS_USER }, /* Allow ordinary user to mount (mtab) */
- { "nouser", MNT_MS_USER, MNT_INVERT | MNT_NOMTAB }, /* Forbid ordinary user to mount */
+ { "user[=]", MNT_MS_USER, MNT_NOHLPS}, /* Allow ordinary user to mount (mtab) */
+ { "nouser", MNT_MS_USER, MNT_INVERT | MNT_NOMTAB | MNT_NOHLPS}, /* Forbid ordinary user to mount */
- { "users", MNT_MS_USERS, MNT_NOMTAB }, /* Allow ordinary users to mount */
- { "nousers", MNT_MS_USERS, MNT_INVERT | MNT_NOMTAB }, /* Forbid ordinary users to mount */
+ { "users", MNT_MS_USERS, MNT_NOMTAB | MNT_NOHLPS}, /* Allow ordinary users to mount */
+ { "nousers", MNT_MS_USERS, MNT_INVERT | MNT_NOMTAB | MNT_NOHLPS}, /* Forbid ordinary users to mount */
{ "owner", MNT_MS_OWNER, MNT_NOMTAB }, /* Let the owner of the device mount */
{ "noowner", MNT_MS_OWNER, MNT_INVERT | MNT_NOMTAB }, /* Device owner has no special privs */
@@ -180,7 +180,7 @@ static const struct libmnt_optmap userspace_opts_map[] =
{ "sizelimit=", MNT_MS_SIZELIMIT, MNT_NOHLPS | MNT_NOMTAB }, /* loop device size limit */
{ "encryption=", MNT_MS_ENCRYPTION, MNT_NOHLPS | MNT_NOMTAB }, /* loop device encryption */
- { "nofail", MNT_MS_NOFAIL, MNT_NOMTAB }, /* Do not fail if ENOENT on dev */
+ { "nofail", MNT_MS_NOFAIL, MNT_NOMTAB | MNT_NOHLPS}, /* Do not fail if ENOENT on dev */
{ "uhelper=", MNT_MS_UHELPER }, /* /sbin/umount.<helper> */
Thanks,
Alan
^ permalink raw reply related
* zram: Compressed RAM-based block devices
From: Soul-Rebel @ 2024-10-01 15:46 UTC (permalink / raw)
To: util-linux, ngupta
Hi hello,
REF: https://docs.kernel.org/admin-guide/blockdev/zram.html
Reading over the document explaining zram and see some spelling errors
in step 4 of the usage section. the word dictionary was misspelled in
the path example "/etc/dictioary"
#pass path to pre-trained zstd dictionary
echo "algo=zstd dict=/etc/dictioary" > /sys/block/zram0/algorithm_params
#same, but using algorithm priority
echo "priority=1 dict=/etc/dictioary" > \
/sys/block/zram0/algorithm_params
#pass path to pre-trained zstd dictionary and compression level
echo "algo=zstd level=8 dict=/etc/dictioary" > \
/sys/block/zram0/algorithm_params
Regards
^ permalink raw reply
* Re: [PATCH 0/2] Add options to enable building lsblk and dmesg
From: Karel Zak @ 2024-09-24 12:32 UTC (permalink / raw)
To: Henrik Lindström; +Cc: util-linux
In-Reply-To: <20240921082310.232867-1-henrik@lxm.se>
On Sat, Sep 21, 2024 at 10:23:08AM GMT, Henrik Lindström wrote:
> Henrik Lindström (2):
> lsblk: allow enabling with --disable-all-programs
> dmesg: allow enabling with --disable-all-programs
>
> configure.ac | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
Applied, thanks!
Karel
>
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* [PATCH 0/2] Add options to enable building lsblk and dmesg
From: Henrik Lindström @ 2024-09-21 8:23 UTC (permalink / raw)
To: util-linux; +Cc: Henrik Lindström
When using --disable-all-programs, it was not possible to build lsblk or
dmesg since no configure arguments existed for enabling them.
Henrik Lindström (2):
lsblk: allow enabling with --disable-all-programs
dmesg: allow enabling with --disable-all-programs
configure.ac | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
--
2.39.5
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox