* [PATCH v2 1/3] write: correctly handle wide characters
[not found] <20230315141649.d7d4ybcg3evyelpo@ws.net.home>
@ 2023-03-15 15:16 ` наб
2023-03-17 10:50 ` Karel Zak
2023-03-15 15:16 ` [PATCH v2 2/3] wall: convert homebrew buffering to open_memstream() наб
2023-03-15 15:16 ` [PATCH v2 3/3] wall: use fputs_careful() наб
2 siblings, 1 reply; 4+ messages in thread
From: наб @ 2023-03-15 15:16 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
[-- Attachment #1: Type: text/plain, Size: 5850 bytes --]
Do this by replacing fputc_careful() (notice that the description said
it's locale-aware ‒ it very much is /not/), with a fputs_careful() which
does the same thing, but if it were to output a byte in the \123 format,
first it checks whether this byte starts a valid multibyte character.
If it does, and that character is printable, write it verbatim.
This means that
echo 'foo åäö ąęćźżń bar' | write nabijaczleweli pts/4
instead of
foo \303\245\303\244\303\266
\304\205\304\231\304\207\305\272\305\274\305\204 bar
yields
foo åäö ąęćźżń bar
or, more realistically, from a message I got earlier today,
Filip powiedzia\305\202 \305\274e zap\305\202aci jutro
becomes
Filip powiedział że zapłaci jutro
Invalid/non-printable sequences get processed as before.
Line reading in write must become getline() to avoid dealing with
partial characters: for example on input consisting solely of
ąęćźżń, where every {1} is an instance, the output would be
{42}ąęć\305\272żń{84}ąęćź\305\274ń{84}ąęćźż\305\204{39}
with just fixed-512 fgets()
Bug-Debian: https://bugs.debian.org/826596
---
include/carefulputc.h | 60 +++++++++++++++++++++++++++++++------------
login-utils/last.c | 4 +--
term-utils/write.c | 25 +++++-------------
3 files changed, 52 insertions(+), 37 deletions(-)
diff --git a/include/carefulputc.h b/include/carefulputc.h
index 8860b1234..740add68e 100644
--- a/include/carefulputc.h
+++ b/include/carefulputc.h
@@ -1,31 +1,59 @@
#ifndef UTIL_LINUX_CAREFULPUTC_H
#define UTIL_LINUX_CAREFULPUTC_H
-/*
- * A putc() for use in write and wall (that sometimes are sgid tty).
- * It avoids control characters in our locale, and also ASCII control
- * characters. Note that the locale of the recipient is unknown.
-*/
#include <stdio.h>
#include <string.h>
#include <ctype.h>
+#ifdef HAVE_WIDECHAR
+#include <wctype.h>
+#endif
+#include <stdbool.h>
#include "cctype.h"
-static inline int fputc_careful(int c, FILE *fp, const char fail)
+/*
+ * A puts() for use in write and wall (that sometimes are sgid tty).
+ * It avoids control and invalid characters.
+ * The locale of the recipient is nominally unknown,
+ * but it's a solid bet that the encoding is compatible with the author's.
+ */
+static inline int fputs_careful(const char * s, FILE *fp, const char ctrl, bool cr_lf)
{
- int ret;
+ int ret = 0;
- if (isprint(c) || c == '\a' || c == '\t' || c == '\r' || c == '\n')
- ret = putc(c, fp);
- else if (!c_isascii(c))
- ret = fprintf(fp, "\\%3o", (unsigned char)c);
- else {
- ret = putc(fail, fp);
- if (ret != EOF)
- ret = putc(c ^ 0x40, fp);
+ for (size_t slen = strlen(s); *s; ++s, --slen) {
+ if (*s == '\n')
+ ret = fputs(cr_lf ? "\r\n" : "\n", fp);
+ else if (isprint(*s) || *s == '\a' || *s == '\t' || *s == '\r')
+ ret = putc(*s, fp);
+ else if (!c_isascii(*s)) {
+#ifdef HAVE_WIDECHAR
+ wchar_t w;
+ size_t clen = mbtowc(&w, s, slen);
+ switch(clen) {
+ case (size_t)-2: // incomplete
+ case (size_t)-1: // EILSEQ
+ mbtowc(NULL, NULL, 0);
+ nonprint:
+ ret = fprintf(fp, "\\%3hho", *s);
+ break;
+ default:
+ if(!iswprint(w))
+ goto nonprint;
+ ret = fwrite(s, 1, clen, fp);
+ s += clen - 1;
+ slen -= clen - 1;
+ break;
+ }
+#else
+ ret = fprintf(fp, "\\%3hho", *s);
+#endif
+ } else
+ ret = fputs((char[]){ ctrl, *s ^ 0x40, '\0' }, fp);
+ if (ret < 0)
+ return EOF;
}
- return (ret < 0) ? EOF : 0;
+ return 0;
}
static inline void fputs_quoted_case(const char *data, FILE *out, int dir)
diff --git a/login-utils/last.c b/login-utils/last.c
index d3eeed4b6..1b45dbf24 100644
--- a/login-utils/last.c
+++ b/login-utils/last.c
@@ -392,7 +392,6 @@ static int list(const struct last_control *ctl, struct utmpx *p, time_t logout_t
char final[512];
char utline[sizeof(p->ut_line) + 1];
char domain[256];
- char *s;
int mins, hours, days;
int r, len;
struct last_timefmt *fmt;
@@ -548,8 +547,7 @@ static int list(const struct last_control *ctl, struct utmpx *p, time_t logout_t
/*
* Print out "final" string safely.
*/
- for (s = final; *s; s++)
- fputc_careful(*s, stdout, '*');
+ fputs_careful(final, stdout, '*', false);
if (len < 0 || (size_t)len >= sizeof(final))
putchar('\n');
diff --git a/term-utils/write.c b/term-utils/write.c
index 8b86e9a9d..b485e28fd 100644
--- a/term-utils/write.c
+++ b/term-utils/write.c
@@ -223,21 +223,6 @@ static void signal_handler(int signo)
signal_received = signo;
}
-/*
- * write_line - like fputs(), but makes control characters visible and
- * turns \n into \r\n.
- */
-static void write_line(char *s)
-{
- while (*s) {
- const int c = *s++;
-
- if ((c == '\n' && fputc_careful('\r', stdout, '^') == EOF)
- || fputc_careful(c, stdout, '^') == EOF)
- err(EXIT_FAILURE, _("carefulputc failed"));
- }
-}
-
/*
* do_write - actually make the connection
*/
@@ -247,7 +232,8 @@ static void do_write(const struct write_control *ctl)
struct passwd *pwd;
time_t now;
struct tm *tm;
- char *host, line[512];
+ char *host, *line = NULL;
+ size_t linelen = 0;
struct sigaction sigact;
/* Determine our login name(s) before the we reopen() stdout */
@@ -286,11 +272,14 @@ static void do_write(const struct write_control *ctl)
free(host);
printf("\r\n");
- while (fgets(line, sizeof(line), stdin) != NULL) {
+ while (getline(&line, &linelen, stdin) >= 0) {
if (signal_received)
break;
- write_line(line);
+
+ if (fputs_careful(line, stdout, '^', true) == EOF)
+ err(EXIT_FAILURE, _("carefulputc failed"));
}
+ free(line);
printf("EOF\r\n");
}
--
2.30.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/3] wall: convert homebrew buffering to open_memstream()
[not found] <20230315141649.d7d4ybcg3evyelpo@ws.net.home>
2023-03-15 15:16 ` [PATCH v2 1/3] write: correctly handle wide characters наб
@ 2023-03-15 15:16 ` наб
2023-03-15 15:16 ` [PATCH v2 3/3] wall: use fputs_careful() наб
2 siblings, 0 replies; 4+ messages in thread
From: наб @ 2023-03-15 15:16 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
[-- Attachment #1: Type: text/plain, Size: 4506 bytes --]
The struct buffer system duplicates a plethora of standard I/O
functions (including a fork of fputc_careful())
and adds a lot of complexity ‒ open_memstream() is standard,
and fits perfectly into this niche
---
term-utils/wall.c | 95 ++++++++++-------------------------------------
1 file changed, 20 insertions(+), 75 deletions(-)
diff --git a/term-utils/wall.c b/term-utils/wall.c
index c601d3e5b..a51a92829 100644
--- a/term-utils/wall.c
+++ b/term-utils/wall.c
@@ -274,74 +274,22 @@ int main(int argc, char **argv)
exit(EXIT_SUCCESS);
}
-struct buffer {
- size_t sz;
- size_t used;
- char *data;
-};
-
-static void buf_enlarge(struct buffer *bs, size_t len)
+static void buf_putc_careful(FILE *fs, int c)
{
- if (bs->sz == 0 || len > bs->sz - bs->used) {
- bs->sz += len < 128 ? 128 : len;
- bs->data = xrealloc(bs->data, bs->sz);
- }
-}
-
-static void buf_puts(struct buffer *bs, const char *s)
-{
- size_t len = strlen(s);
-
- buf_enlarge(bs, len + 1);
- memcpy(bs->data + bs->used, s, len + 1);
- bs->used += len;
-}
-
-static void __attribute__((__format__ (__printf__, 2, 3)))
- buf_printf(struct buffer *bs, const char *fmt, ...)
-{
- int rc;
- va_list ap;
- size_t limit;
-
- buf_enlarge(bs, 0); /* default size */
- limit = bs->sz - bs->used;
-
- va_start(ap, fmt);
- rc = vsnprintf(bs->data + bs->used, limit, fmt, ap);
- va_end(ap);
-
- if (rc >= 0 && (size_t) rc >= limit) { /* not enough, enlarge */
- buf_enlarge(bs, (size_t)rc + 1);
- limit = bs->sz - bs->used;
- va_start(ap, fmt);
- rc = vsnprintf(bs->data + bs->used, limit, fmt, ap);
- va_end(ap);
- }
-
- if (rc > 0)
- bs->used += rc;
-}
-
-static void buf_putc_careful(struct buffer *bs, int c)
-{
- if (isprint(c) || c == '\a' || c == '\t' || c == '\r' || c == '\n') {
- buf_enlarge(bs, 1);
- bs->data[bs->used++] = c;
- } else if (!c_isascii(c))
- buf_printf(bs, "\\%3o", (unsigned char)c);
- else {
- char tmp[] = { '^', c ^ 0x40, '\0' };
- buf_puts(bs, tmp);
- }
+ if (isprint(c) || c == '\a' || c == '\t' || c == '\r' || c == '\n')
+ fputc(c, fs);
+ else if (!c_isascii(c))
+ fprintf(fs, "\\%3o", (unsigned char)c);
+ else
+ fputs((char[]){ '^', c ^ 0x40, '\0' }, fs);
}
static char *makemsg(char *fname, char **mvec, int mvecsz,
size_t *mbufsize, int print_banner)
{
- struct buffer _bs = {.used = 0}, *bs = &_bs;
register int ch, cnt;
- char *p, *lbuf;
+ char *p, *lbuf, *retbuf;
+ FILE * fs = open_memstream(&retbuf, mbufsize);
long line_max;
line_max = sysconf(_SC_LINE_MAX);
@@ -379,15 +327,15 @@ static char *makemsg(char *fname, char **mvec, int mvecsz,
*/
/* snprintf is not always available, but the sprintf's here
will not overflow as long as %d takes at most 100 chars */
- buf_printf(bs, "\r%*s\r\n", TERM_WIDTH, " ");
+ fprintf(fs, "\r%*s\r\n", TERM_WIDTH, " ");
snprintf(lbuf, line_max,
_("Broadcast message from %s@%s (%s) (%s):"),
whom, hostname, where, date);
- buf_printf(bs, "%-*.*s\007\007\r\n", TERM_WIDTH, TERM_WIDTH, lbuf);
+ fprintf(fs, "%-*.*s\007\007\r\n", TERM_WIDTH, TERM_WIDTH, lbuf);
free(hostname);
}
- buf_printf(bs, "%*s\r\n", TERM_WIDTH, " ");
+ fprintf(fs, "%*s\r\n", TERM_WIDTH, " ");
if (mvec) {
/*
@@ -396,11 +344,11 @@ static char *makemsg(char *fname, char **mvec, int mvecsz,
int i;
for (i = 0; i < mvecsz; i++) {
- buf_puts(bs, mvec[i]);
+ fputs(mvec[i], fs);
if (i < mvecsz - 1)
- buf_puts(bs, " ");
+ fputc(' ', fs);
}
- buf_puts(bs, "\r\n");
+ fputs("\r\n", fs);
} else {
/*
* read message from <file>
@@ -428,23 +376,20 @@ static char *makemsg(char *fname, char **mvec, int mvecsz,
while (fgets(lbuf, line_max, stdin)) {
for (cnt = 0, p = lbuf; (ch = *p) != '\0'; ++p, ++cnt) {
if (cnt == TERM_WIDTH || ch == '\n') {
- for (; cnt < TERM_WIDTH; ++cnt)
- buf_puts(bs, " ");
- buf_puts(bs, "\r\n");
+ fprintf(fs, "%*s\r\n", TERM_WIDTH - cnt, "");
cnt = 0;
}
if (ch == '\t')
cnt += (7 - (cnt % 8));
if (ch != '\n')
- buf_putc_careful(bs, ch);
+ buf_putc_careful(fs, ch);
}
}
}
- buf_printf(bs, "%*s\r\n", TERM_WIDTH, " ");
+ fprintf(fs, "%*s\r\n", TERM_WIDTH, " ");
free(lbuf);
- bs->data[bs->used] = '\0'; /* be paranoid */
- *mbufsize = bs->used;
- return bs->data;
+ fclose(fs);
+ return retbuf;
}
--
2.30.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 3/3] wall: use fputs_careful()
[not found] <20230315141649.d7d4ybcg3evyelpo@ws.net.home>
2023-03-15 15:16 ` [PATCH v2 1/3] write: correctly handle wide characters наб
2023-03-15 15:16 ` [PATCH v2 2/3] wall: convert homebrew buffering to open_memstream() наб
@ 2023-03-15 15:16 ` наб
2 siblings, 0 replies; 4+ messages in thread
From: наб @ 2023-03-15 15:16 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
[-- Attachment #1: Type: text/plain, Size: 6867 bytes --]
LINE_MAX only applies to teletypes in canonical mode: when stdin is a
file, it could still very much tear; start off at 512 for the sprintf(),
then use getline() like in write.
The line wrapping has one suboptimal edge-case:
$ wall < all
Broadcast message from nabijaczleweli@tarta (pts/4) (Tue Mar 14 22:31:25
2023):
^N^O^P^Q^R^S^T^U^V^W^X^Y^Z^[^\^]^^^_
!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJ
KLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~^?\200\201\202\203\204\205\206
\207\210\211\212\213\214\215\216\217\220\221\222\223\224\225\226\227\230\231\232
\233\234\235\236\237\240\241\242\243\244\245\246\247\250\251\252\253\254\255\256
\257\260\261\262\263\264\265\266\267\270\271\272\273\274\275\276\277\300\301\302
\303\304\305\306\307\310\311\312\313\314\315\316\317\320\321\322\323\324\325\326
\327\330\331\332\333\334\335\336\337\340\341\342\343\344\345\346\347\350\351\352
\353\354\355\356\357\360\361\362\363\364\365\366\367\370\371\372\373\374\375\376
\377
but that's a pathological input, and the result is still infinitely
better than it was before, so fixing that is more trouble than it's
worth.
Bug-Debian: https://bugs.debian.org/826596
---
include/carefulputc.h | 42 +++++++++++++++++++++++++++++++++---------
login-utils/last.c | 2 +-
term-utils/wall.c | 38 ++++++--------------------------------
term-utils/write.c | 2 +-
4 files changed, 41 insertions(+), 43 deletions(-)
diff --git a/include/carefulputc.h b/include/carefulputc.h
index 740add68e..3cc6f7ff9 100644
--- a/include/carefulputc.h
+++ b/include/carefulputc.h
@@ -6,6 +6,7 @@
#include <ctype.h>
#ifdef HAVE_WIDECHAR
#include <wctype.h>
+#include <wchar.h>
#endif
#include <stdbool.h>
@@ -15,18 +16,35 @@
* A puts() for use in write and wall (that sometimes are sgid tty).
* It avoids control and invalid characters.
* The locale of the recipient is nominally unknown,
- * but it's a solid bet that the encoding is compatible with the author's.
+ * but it's a solid bet that it's compatible with the author's.
+ * Use soft_width=0 to disable wrapping.
*/
-static inline int fputs_careful(const char * s, FILE *fp, const char ctrl, bool cr_lf)
+static inline int fputs_careful(const char * s, FILE *fp, const char ctrl, bool cr_lf, int soft_width)
{
- int ret = 0;
+ int ret = 0, col = 0;
for (size_t slen = strlen(s); *s; ++s, --slen) {
- if (*s == '\n')
+ if (*s == '\t')
+ col += (7 - (col % 8)) - 1;
+ else if (*s == '\r')
+ col = -1;
+ else if (*s == '\a')
+ --col;
+
+ if ((soft_width && col >= soft_width) || *s == '\n') {
+ if (soft_width) {
+ fprintf(fp, "%*s", soft_width - col, "");
+ col = 0;
+ }
ret = fputs(cr_lf ? "\r\n" : "\n", fp);
- else if (isprint(*s) || *s == '\a' || *s == '\t' || *s == '\r')
+ if (*s == '\n' || ret < 0)
+ goto wrote;
+ }
+
+ if (isprint(*s) || *s == '\a' || *s == '\t' || *s == '\r') {
ret = putc(*s, fp);
- else if (!c_isascii(*s)) {
+ ++col;
+ } else if (!c_isascii(*s)) {
#ifdef HAVE_WIDECHAR
wchar_t w;
size_t clen = mbtowc(&w, s, slen);
@@ -35,21 +53,27 @@ static inline int fputs_careful(const char * s, FILE *fp, const char ctrl, bool
case (size_t)-1: // EILSEQ
mbtowc(NULL, NULL, 0);
nonprint:
- ret = fprintf(fp, "\\%3hho", *s);
+ col += ret = fprintf(fp, "\\%3hho", *s);
break;
default:
if(!iswprint(w))
goto nonprint;
ret = fwrite(s, 1, clen, fp);
+ if (soft_width)
+ col += wcwidth(w);
s += clen - 1;
slen -= clen - 1;
break;
}
#else
- ret = fprintf(fp, "\\%3hho", *s);
+ col += ret = fprintf(fp, "\\%3hho", *s);
#endif
- } else
+ } else {
ret = fputs((char[]){ ctrl, *s ^ 0x40, '\0' }, fp);
+ col += 2;
+ }
+
+ wrote:
if (ret < 0)
return EOF;
}
diff --git a/login-utils/last.c b/login-utils/last.c
index 1b45dbf24..37c6abe97 100644
--- a/login-utils/last.c
+++ b/login-utils/last.c
@@ -547,7 +547,7 @@ static int list(const struct last_control *ctl, struct utmpx *p, time_t logout_t
/*
* Print out "final" string safely.
*/
- fputs_careful(final, stdout, '*', false);
+ fputs_careful(final, stdout, '*', false, 0);
if (len < 0 || (size_t)len >= sizeof(final))
putchar('\n');
diff --git a/term-utils/wall.c b/term-utils/wall.c
index a51a92829..377db4518 100644
--- a/term-utils/wall.c
+++ b/term-utils/wall.c
@@ -274,29 +274,13 @@ int main(int argc, char **argv)
exit(EXIT_SUCCESS);
}
-static void buf_putc_careful(FILE *fs, int c)
-{
- if (isprint(c) || c == '\a' || c == '\t' || c == '\r' || c == '\n')
- fputc(c, fs);
- else if (!c_isascii(c))
- fprintf(fs, "\\%3o", (unsigned char)c);
- else
- fputs((char[]){ '^', c ^ 0x40, '\0' }, fs);
-}
-
static char *makemsg(char *fname, char **mvec, int mvecsz,
size_t *mbufsize, int print_banner)
{
- register int ch, cnt;
- char *p, *lbuf, *retbuf;
+ char *lbuf, *retbuf;
FILE * fs = open_memstream(&retbuf, mbufsize);
- long line_max;
-
- line_max = sysconf(_SC_LINE_MAX);
- if (line_max <= 0)
- line_max = 512;
-
- lbuf = xmalloc(line_max);
+ size_t lbuflen = 512;
+ lbuf = xmalloc(lbuflen);
if (print_banner == TRUE) {
char *hostname = xgethostname();
@@ -329,7 +313,7 @@ static char *makemsg(char *fname, char **mvec, int mvecsz,
will not overflow as long as %d takes at most 100 chars */
fprintf(fs, "\r%*s\r\n", TERM_WIDTH, " ");
- snprintf(lbuf, line_max,
+ snprintf(lbuf, lbuflen,
_("Broadcast message from %s@%s (%s) (%s):"),
whom, hostname, where, date);
fprintf(fs, "%-*.*s\007\007\r\n", TERM_WIDTH, TERM_WIDTH, lbuf);
@@ -373,18 +357,8 @@ static char *makemsg(char *fname, char **mvec, int mvecsz,
/*
* Read message from stdin.
*/
- while (fgets(lbuf, line_max, stdin)) {
- for (cnt = 0, p = lbuf; (ch = *p) != '\0'; ++p, ++cnt) {
- if (cnt == TERM_WIDTH || ch == '\n') {
- fprintf(fs, "%*s\r\n", TERM_WIDTH - cnt, "");
- cnt = 0;
- }
- if (ch == '\t')
- cnt += (7 - (cnt % 8));
- if (ch != '\n')
- buf_putc_careful(fs, ch);
- }
- }
+ while (getline(&lbuf, &lbuflen, stdin) >= 0)
+ fputs_careful(lbuf, fs, '^', true, TERM_WIDTH);
}
fprintf(fs, "%*s\r\n", TERM_WIDTH, " ");
diff --git a/term-utils/write.c b/term-utils/write.c
index b485e28fd..a5a21280c 100644
--- a/term-utils/write.c
+++ b/term-utils/write.c
@@ -276,7 +276,7 @@ static void do_write(const struct write_control *ctl)
if (signal_received)
break;
- if (fputs_careful(line, stdout, '^', true) == EOF)
+ if (fputs_careful(line, stdout, '^', true, 0) == EOF)
err(EXIT_FAILURE, _("carefulputc failed"));
}
free(line);
--
2.30.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread