* [RFC PATCH util-linux] text-utils: add bits command
@ 2024-10-16 20:26 Robin Jarry
2024-10-16 21:00 ` Thomas Weißschuh
2024-10-16 21:57 ` [PATCH util-linux v2] " Robin Jarry
0 siblings, 2 replies; 5+ messages in thread
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 [flat|nested] 5+ messages in thread
* Re: [RFC PATCH util-linux] text-utils: add bits command
2024-10-16 20:26 [RFC PATCH util-linux] text-utils: add bits command Robin Jarry
@ 2024-10-16 21:00 ` Thomas Weißschuh
2024-10-16 21:11 ` Robin Jarry
2024-10-16 21:57 ` [PATCH util-linux v2] " Robin Jarry
1 sibling, 1 reply; 5+ messages in thread
From: Thomas Weißschuh @ 2024-10-16 21:00 UTC (permalink / raw)
To: Robin Jarry; +Cc: util-linux
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 [flat|nested] 5+ messages in thread
* Re: [RFC PATCH util-linux] text-utils: add bits command
2024-10-16 21:00 ` Thomas Weißschuh
@ 2024-10-16 21:11 ` Robin Jarry
2024-10-18 6:53 ` Thomas Weißschuh
0 siblings, 1 reply; 5+ messages in thread
From: Robin Jarry @ 2024-10-16 21:11 UTC (permalink / raw)
To: Thomas Weißschuh; +Cc: util-linux
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 [flat|nested] 5+ messages in thread
* [PATCH util-linux v2] text-utils: add bits command
2024-10-16 20:26 [RFC PATCH util-linux] text-utils: add bits command Robin Jarry
2024-10-16 21:00 ` Thomas Weißschuh
@ 2024-10-16 21:57 ` Robin Jarry
1 sibling, 0 replies; 5+ messages in thread
From: Robin Jarry @ 2024-10-16 21:57 UTC (permalink / raw)
To: util-linux; +Cc: Thomas Weißschuh
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 [flat|nested] 5+ messages in thread
* Re: [RFC PATCH util-linux] text-utils: add bits command
2024-10-16 21:11 ` Robin Jarry
@ 2024-10-18 6:53 ` Thomas Weißschuh
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Weißschuh @ 2024-10-18 6:53 UTC (permalink / raw)
To: Robin Jarry; +Cc: util-linux
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 [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-18 6:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-16 20:26 [RFC PATCH util-linux] text-utils: add bits command Robin Jarry
2024-10-16 21:00 ` Thomas Weißschuh
2024-10-16 21:11 ` Robin Jarry
2024-10-18 6:53 ` Thomas Weißschuh
2024-10-16 21:57 ` [PATCH util-linux v2] " Robin Jarry
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).