On Sun, Jan 30, 2022 at 10:23:43AM -0600, Scott Cheloha wrote:
> In tr(1), we have these two global arrays, "string1" and "string2".
>
> I have a few complaints:
>
> 1. They are not strings. They are lookup tables. The names are
> misleading.
>
> 2. The arguments given to tr(1) in argv[] are indeed called "string1"
> and "string2". These are the names used in the standard, the manpage,
> and the usage printout.
>
> However, the lookup tables are merely *described* by these arguments.
> They are not the arguments themselves.
>
> 3. The meaning of the contents of these lookup tables changes depending
> on which of the five different operating modes tr(1) is running in.
>
> string1[i] might mean "delete byte i" or "squeeze byte i" or
> "replace byte i with the value string1[i]" depending on how
> tr(1) was invoked.
>
> Given this, I think it'd be a lot nicer if we named the tables to
> indicate which transformation operation they correspond to.
>
> We have three such operations: "delete", "squeeze", and "translate".
> So we ought to have a table for each. And in setup() we should call
> the table a "table", not a "string".
>
> Now when you look at the loops in main() you can immediately
> understand which operation is happening without needing to consult the
> manpage or the comments. (Seriously, look.)
>
> I have more cleanup I want to do here in tr.c, but I think renaming
> these tables first is going to make the rest of it a lot easier to
> review.
>
> ok?
1 week bump.
ok?
Index: tr.c
===================================================================
RCS file: /cvs/src/usr.bin/tr/tr.c,v
retrieving revision 1.20
diff -u -p -r1.20 tr.c
--- tr.c 2 Nov 2021 15:45:52 -0000 1.20
+++ tr.c 30 Jan 2022 16:14:21 -0000
@@ -40,7 +40,8 @@
#include "extern.h"
-static int string1[NCHARS] = {
+int delete[NCHARS], squeeze[NCHARS];
+int translate[NCHARS] = {
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, /* ASCII */
0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
@@ -73,7 +74,7 @@ static int string1[NCHARS] = {
0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef,
0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff,
-}, string2[NCHARS];
+};
STR s1 = { STRING1, NORMAL, 0, OOBCH, { 0, OOBCH }, NULL, NULL };
STR s2 = { STRING2, NORMAL, 0, OOBCH, { 0, OOBCH }, NULL, NULL };
@@ -122,11 +123,11 @@ main(int argc, char *argv[])
if (argc != 2)
usage();
- setup(string1, argv[0], &s1, cflag);
- setup(string2, argv[1], &s2, 0);
+ setup(delete, argv[0], &s1, cflag);
+ setup(squeeze, argv[1], &s2, 0);
for (lastch = OOBCH; (ch = getchar()) != EOF;)
- if (!string1[ch] && (!string2[ch] || lastch != ch)) {
+ if (!delete[ch] && (!squeeze[ch] || lastch != ch)) {
lastch = ch;
(void)putchar(ch);
}
@@ -141,10 +142,10 @@ main(int argc, char *argv[])
if (argc != 1)
usage();
- setup(string1, argv[0], &s1, cflag);
+ setup(delete, argv[0], &s1, cflag);
while ((ch = getchar()) != EOF)
- if (!string1[ch])
+ if (!delete[ch])
(void)putchar(ch);
exit(0);
}
@@ -154,10 +155,10 @@ main(int argc, char *argv[])
* Squeeze all characters (or complemented characters) in string1.
*/
if (sflag && argc == 1) {
- setup(string1, argv[0], &s1, cflag);
+ setup(squeeze, argv[0], &s1, cflag);
for (lastch = OOBCH; (ch = getchar()) != EOF;)
- if (!string1[ch] || lastch != ch) {
+ if (!squeeze[ch] || lastch != ch) {
lastch = ch;
(void)putchar(ch);
}
@@ -177,7 +178,7 @@ main(int argc, char *argv[])
s2.str = (unsigned char *)argv[1];
if (cflag)
- for (cnt = NCHARS, p = string1; cnt--;)
+ for (cnt = NCHARS, p = translate; cnt--;)
*p++ = OOBCH;
if (!next(&s2))
@@ -187,45 +188,45 @@ main(int argc, char *argv[])
ch = s2.lastch;
if (sflag)
while (next(&s1)) {
- string1[s1.lastch] = ch = s2.lastch;
- string2[ch] = 1;
+ translate[s1.lastch] = ch = s2.lastch;
+ squeeze[ch] = 1;
(void)next(&s2);
}
else
while (next(&s1)) {
- string1[s1.lastch] = ch = s2.lastch;
+ translate[s1.lastch] = ch = s2.lastch;
(void)next(&s2);
}
if (cflag)
- for (cnt = 0, p = string1; cnt < NCHARS; ++p, ++cnt)
+ for (cnt = 0, p = translate; cnt < NCHARS; ++p, ++cnt)
*p = *p == OOBCH ? ch : cnt;
if (sflag)
for (lastch = OOBCH; (ch = getchar()) != EOF;) {
- ch = string1[ch];
- if (!string2[ch] || lastch != ch) {
+ ch = translate[ch];
+ if (!squeeze[ch] || lastch != ch) {
lastch = ch;
(void)putchar(ch);
}
}
else
while ((ch = getchar()) != EOF)
- (void)putchar(string1[ch]);
+ (void)putchar(translate[ch]);
exit (0);
}
static void
-setup(int *string, char *arg, STR *str, int cflag)
+setup(int *table, char *arg, STR *str, int cflag)
{
int cnt, *p;
str->str = (unsigned char *)arg;
- bzero(string, NCHARS * sizeof(int));
+ bzero(table, NCHARS * sizeof(int));
while (next(str))
- string[str->lastch] = 1;
+ table[str->lastch] = 1;
if (cflag)
- for (p = string, cnt = NCHARS; cnt--; ++p)
+ for (p = table, cnt = NCHARS; cnt--; ++p)
*p = !*p;
}
