Re: [PATCH 1/2] less: fully implement -R and print color escapes
Actually, after sleeping on it, I think count_colctrl could use some more validation (even though even less doesn't do much more than this). v2 incoming. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 2/2] less: replace most uses of NORMAL escape with UNHIGHLIGHT
On Fri, 15 Apr 2022 at 05:49, Kang-Che Sung wrote: > What's the difference between the NORMAL escape and the UNHIGHLIGHT escape? > Is there a test case to demonstrate th> The UNHIGHLIGHT escape flips only the HIGHLIGHT (invert, actually) bit back to off. The normal escape flips ALL special display bits back to off, including color. We don't want that in case there is a highlighted term (e.g. another control sequence) in between a colored line. test case: printf '\033[33mhello\033\007\013hi\033[m' > ./tmp Compare how busybox less without this 2nd patch vs. greenwood less (and bbless with this patch) shows the output. In the former, the colored line is just cut short because of the highlighted control chars. In the latter, the entire line is colored, with small highlighted (colored) sections in between. One thing this cannot deal with is a highlight escape in the input itself, which would be cut short by any less-emmitted highlight sequence. Avoiding that would probably require looking at the escapes and keeping state. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 2/2] less: replace most uses of NORMAL escape with UNHIGHLIGHT
On Friday, April 15, 2022, FriendlyNeighborhoodShane < shane.880088.s...@gmail.com> wrote: > Fixes conflict when -R's color escape codes are mixed together with > highlights. Better complement to HIGHLIGHT. What's the difference between the NORMAL escape and the UNHIGHLIGHT escape? Is there a test case to demonstrate the difference? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v8] seedrng: import SeedRNG utility for kernel RNG seed files
Hi Bernhard, On Tue, Apr 12, 2022 at 8:37 PM Bernhard Reutner-Fischer wrote: > > Hi Jason! > I'm a bit surprised that even if i give -n the seed is moved to > seed.credit. The next boot/run will find the now creditable seed and > happily add it, IIUC, despite i wanted it to not be credited? > Is this intentional? Yes. You misunderstand the purpose of the utility. It creates a creditable seed when the kernel is able to produce safe random numbers. In that case, the creditability or non-creditability of the previous seed does not matter. > > PPS: I'm attaching some fiddle on top of your v8 which would give a > relative savings of I'm not _super_ comfortable with all of these, and I don't know if I feel good about attaching them to the original commit. At this late-stage of golfing, we really risk introducing excessively brittle code and bringing along new bugs with it. These additional strokes need to be considered very carefully and individually. So I think at this point, I'm done on the green, and I think what you ought to do is commit my v8, and then send your patches as a series, and I'll take the time to look very carefully at each one individually and comment on them. And by keeping those as separate commits, if we both do miss something, it'll be easier to revert and for others to notice the error too. Does that plan sound okay to you? Commit v8, and then tee off a new golfing series? Jason ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v8] seedrng: import SeedRNG utility for kernel RNG seed files
Hi Jason!
On Sun, 10 Apr 2022 23:20:34 +0200
"Jason A. Donenfeld" wrote:
> +static int read_new_seed(uint8_t *seed, size_t len, bool *is_creditable)
> +{
> + ssize_t ret;
> +
> + *is_creditable = false;
> + ret = getrandom(seed, len, GRND_NONBLOCK);
> + if (ret == (ssize_t)len) {
> + *is_creditable = true;
> + return 0;
> + } else if (ret < 0 && errno == ENOSYS) {
> + struct pollfd random_fd = {
> + .fd = open("/dev/random", O_RDONLY),
> + .events = POLLIN
> + };
> + if (random_fd.fd < 0)
> + return -1;
> + *is_creditable = safe_poll(_fd, 1, 0) == 1;
> + close(random_fd.fd);
> + } else if (getrandom(seed, len, GRND_INSECURE) == (ssize_t)len)
> + return 0;
> + if (open_read_close("/dev/urandom", seed, len) == (ssize_t)len)
> + return 0;
> + return -1;
> +}
> +int seedrng_main(int argc, char *argv[]) MAIN_EXTERNALLY_VISIBLE;
> +int seedrng_main(int argc UNUSED_PARAM, char *argv[])
> +{
> + bool new_seed_creditable;
> + bool skip_credit = false;
> + struct timespec timestamp = { 0 };
> + sha256_ctx_t hash;
> +
> + int opt;
> + enum {
> + OPT_d = (1 << 0),
> + OPT_n = (1 << 1)
> + };
> +#if ENABLE_LONG_OPTS
> + static const char longopts[] ALIGN1 =
> + "seed-dir\0"Required_argument "d"
> + "skip-credit\0" No_argument "n"
> + ;
> +#endif
> +
> + opt = getopt32long(argv, "d:n", longopts, _dir);
> + skip_credit = opt & OPT_n;
> + ret = seed_from_file_if_exists(non_creditable_seed, dfd, false, );
> + if (ret < 0)
> + program_ret |= 1 << 1;
> + ret = seed_from_file_if_exists(creditable_seed, dfd, !skip_credit,
> );
> + printf("Saving %zu bits of %s seed for next boot\n", new_seed_len * 8,
> new_seed_creditable ? "creditable" : "non-creditable");
> + if (new_seed_creditable && rename(non_creditable_seed, creditable_seed)
> < 0) {
I'm a bit surprised that even if i give -n the seed is moved to
seed.credit. The next boot/run will find the now creditable seed and
happily add it, IIUC, despite i wanted it to not be credited?
Is this intentional?
PS: And i can literally hear some security expert to
mknod -m 0666 /dev/random c 1 3;# a /dev/zero, doesn't block much
and then run around complaining that we credited all those zeros ;)
So a paranoid impl would check for them to be c1,8 and c1,9 before
trusting them and crediting, i suppose. But i'd rather want to avoid
these checks in busybox since that's a bit too much bloat. But i
thought i'd note it for your other, bigger impls, fwiw. But you
certainly have that check in there already anyway..
PPS: I'm attaching some fiddle on top of your v8 which would give a
relative savings of
function old new delta
seedrng_main 9481228+280
.rodata 108338 108206-132
seed_from_file_if_exists 410 --410
--
(add/remove: 0/1 grow/shrink: 1/1 up/down: 280/-542) Total: -262 bytes
$ size */seedrng.o*
textdata bss dec hex filename
1900 0 01900 76c util-linux/seedrng.o.v8
1658 0 01658 67a util-linux/seedrng.o
I still have to see if we can make the construction of the two seed
file names a bit smaller. And the headers should be pruned.
thanks and cheers,
>From eb570afb9e1f550b589602f86269ecfe61c979a0 Mon Sep 17 00:00:00 2001
From: Bernhard Reutner-Fischer
Date: Tue, 12 Apr 2022 20:11:21 +0200
Subject: [PATCH 1/8] seedrng: shrink
read_new_seed: The indicator for ENOSYS would be if getrandom does not
return the requested len _and_ errno is ENOSYS. Remove the ret check.
seed_rng: just return the retval of the ioctl.
seedrng_main: no point in looking at the uid, the command should fail in
various ways if we are not allowed to do what we want.
function old new delta
seedrng_main 948 924 -24
seed_from_file_if_exists 410 383 -27
--
(add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-51) Total: -51 bytes
---
util-linux/seedrng.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/util-linux/seedrng.c b/util-linux/seedrng.c
index 5735dc059..d8120f7b3 100644
--- a/util-linux/seedrng.c
+++ b/util-linux/seedrng.c
@@ -95,7 +95,7 @@ static int read_new_seed(uint8_t *seed, size_t len, bool *is_creditable)
if (ret == (ssize_t)len) {
*is_creditable = true;
return 0;
- }
[PATCH 2/2] less: replace most uses of NORMAL escape with UNHIGHLIGHT
Fixes conflict when -R's color escape codes are mixed together with
highlights. Better complement to HIGHLIGHT.
---
miscutils/less.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/miscutils/less.c b/miscutils/less.c
index 392a3ef3c..7fcd6951a 100644
--- a/miscutils/less.c
+++ b/miscutils/less.c
@@ -155,6 +155,7 @@
#define ESC "\033"
/* The escape codes for highlighted and normal text */
#define HIGHLIGHT ESC"[7m"
+#define UNHIGHLIGHT ESC"[27m"
#define NORMAL ESC"[m"
/* The escape code to home and clear to the end of screen */
#define CLEAR ESC"[H"ESC"[J"
@@ -312,13 +313,13 @@ static void clear_line(void)
static void print_hilite(const char *str)
{
- printf(HIGHLIGHT"%s"NORMAL, str);
+ printf(HIGHLIGHT"%s"UNHIGHLIGHT, str);
}
static void print_statusline(const char *str)
{
clear_line();
- printf(HIGHLIGHT"%.*s"NORMAL, width - 1, str);
+ printf(HIGHLIGHT"%.*s"UNHIGHLIGHT, width - 1, str);
}
/* Exit the program gracefully */
@@ -710,7 +711,7 @@ static void m_status_print(void)
percent = (100 * last + num_lines/2) / num_lines;
printf(" %i%%", percent <= 100 ? percent : 100);
}
- printf(NORMAL);
+ printf(UNHIGHLIGHT);
}
#endif
@@ -740,7 +741,7 @@ static void status_print(void)
if (!cur_fline)
p = filename;
if (num_files > 1) {
- printf(HIGHLIGHT"%s (file %i of %i)"NORMAL,
+ printf(HIGHLIGHT"%s (file %i of %i)"UNHIGHLIGHT,
p, current_file, num_files);
return;
}
@@ -808,7 +809,7 @@ static void print_found(const char *line)
/* buf[] holds quarantined version of str */
/* Each part of the line that matches has the HIGHLIGHT
-* and NORMAL escape sequences placed around it.
+* and UNHIGHLIGHT escape sequences placed around it.
* NB: we regex against line, but insert text
* from quarantined copy (buf[]) */
str = buf;
@@ -817,7 +818,7 @@ static void print_found(const char *line)
goto start;
while (match_status == 0) {
- char *new = xasprintf("%s%.*s"HIGHLIGHT"%.*s"NORMAL,
+ char *new = xasprintf("%s%.*s"HIGHLIGHT"%.*s"UNHIGHLIGHT,
growline ? growline : "",
(int)match_structs.rm_so, str,
(int)(match_structs.rm_eo -
match_structs.rm_so),
@@ -1551,7 +1552,7 @@ static void show_flag_status(void)
}
clear_line();
- printf(HIGHLIGHT"The status of the flag is: %u"NORMAL, flag_val != 0);
+ printf(HIGHLIGHT"The status of the flag is: %u"UNHIGHLIGHT, flag_val !=
0);
}
#endif
--
2.35.2
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/2] less: fully implement -R and print color escapes
Emit NORMAL at newlines, like less, to deal with malformed input.
---
Fits a little akwardly into the output flow with a break and continue,
but it works.
miscutils/less.c | 54 +++-
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/miscutils/less.c b/miscutils/less.c
index 8a0525cb7..392a3ef3c 100644
--- a/miscutils/less.c
+++ b/miscutils/less.c
@@ -139,7 +139,7 @@
//usage: "\n -S Truncate long lines"
//usage: )
//usage: IF_FEATURE_LESS_RAW(
-//usage: "\n -R Remove color escape codes in input"
+//usage: "\n -R Keep ANSI color escape codes in input"
//usage: )
//usage: "\n -~ Suppress ~s displayed past EOF"
@@ -229,9 +229,6 @@ struct globals {
regex_t pattern;
smallint pattern_valid;
#endif
-#if ENABLE_FEATURE_LESS_RAW
- smallint in_escape;
-#endif
#if ENABLE_FEATURE_LESS_ASK_TERMINAL
smallint winsize_err;
#endif
@@ -541,26 +538,6 @@ static void read_lines(void)
*--p = '\0';
continue;
}
-#if ENABLE_FEATURE_LESS_RAW
- if (option_mask32 & FLAG_R) {
- if (c == '\033')
- goto discard;
- if (G.in_escape) {
- if (isdigit(c)
-|| c == '['
-|| c == ';'
-|| c == 'm'
- ) {
- discard:
- G.in_escape = (c != 'm');
- readpos++;
- continue;
- }
- /* Hmm, unexpected end of "ESC [ N ; N
m" sequence */
- G.in_escape = 0;
- }
- }
-#endif
{
size_t new_last_line_pos = last_line_pos + 1;
if (c == '\t') {
@@ -780,6 +757,7 @@ static const char ctrlconv[] ALIGN1 =
* '\n' is a former NUL - we subst it with @, not J */
"\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x40\x4b\x4c\x4d\x4e\x4f"
"\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f";
+static const char colctrl[] ALIGN1 = "\x1b[0123456789;m";
static void print_lineno(const char *line)
{
@@ -864,13 +842,32 @@ static void print_found(const char *line)
void print_found(const char *line);
#endif
+static size_t count_colctrl(const char *str) {
+ size_t n = strspn(str, colctrl);
+ // must end with m, need at least '[m'
+ if (n < 3 || *(str+n-1) != 'm')
+ return 0;
+ return n;
+}
+
static void print_ascii(const char *str)
{
char buf[width+1];
char *p;
size_t n;
+#if ENABLE_FEATURE_LESS_RAW
+ size_t esc = 0;
+#endif
while (*str) {
+#if ENABLE_FEATURE_LESS_RAW
+ if (esc) {
+ printf("%.*s", (int) esc, str);
+ str += esc;
+ esc = 0;
+ continue;
+ }
+#endif
n = strcspn(str, controls);
if (n) {
if (!str[n]) break;
@@ -886,6 +883,14 @@ static void print_ascii(const char *str)
/* VT100's CSI, aka Meta-ESC. Who's inventor? */
/* I want to know who committed this sin */
*p++ = '{';
+#if ENABLE_FEATURE_LESS_RAW
+ else if ((option_mask32 & FLAG_R)
+&& *str == '\033'
+// need at least '[m'
+&& (esc = count_colctrl(str))) {
+ break;
+ }
+#endif
else
*p++ = ctrlconv[(unsigned char)*str];
str++;
@@ -894,6 +899,7 @@ static void print_ascii(const char *str)
print_hilite(buf);
}
puts(str);
+ printf(NORMAL);
}
/* Print the buffer */
--
2.35.2
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
