From afb4998dd4de03c2d635faf7db89f3d3f6c3a6ff Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Tue, 15 Nov 2022 21:19:35 -0800 Subject: [PATCH] rs: Use getopt() and strtol() instead of mannual parsing. This uses the "::" extension to getopt() to handle options which take an optional argument. The updated flag tests were all wrong before and only passed because the manual parser failed to raise errors when a required argument was missing. The invalid argument test now gets a better error message. Reviewed by: brooks, imp, emaste Differential Revision: https://reviews.freebsd.org/D36834 --- usr.bin/rs/rs.cc | 247 ++++++++++++++---------------------- usr.bin/rs/tests/rs_test.sh | 11 +- 2 files changed, 104 insertions(+), 154 deletions(-) diff --git a/usr.bin/rs/rs.cc b/usr.bin/rs/rs.cc index c20956b3b5e1..8989288e3ec7 100644 --- a/usr.bin/rs/rs.cc +++ b/usr.bin/rs/rs.cc @@ -41,6 +41,7 @@ #include #include #include +#include #include static long flags; @@ -63,9 +64,6 @@ static long flags; #define NOARGS 0200000 static short *colwidths; -static short *cord; -static short *icbd; -static short *ocbd; static std::vector elem; static char *curline; static size_t curlen; @@ -76,13 +74,12 @@ static int skip; static int propgutter; static char isep = ' ', osep = ' '; static char blank[] = ""; -static int owidth = 80, gutter = 2; +static size_t owidth = 80, gutter = 2; static void getargs(int, char *[]); static void getfile(void); static int get_line(void); -static char *getlist(short **, char *); -static char *getnum(int *, char *, int); +static long getnum(const char *); static void prepfile(void); static void prints(char *, int); static void putfile(void); @@ -120,7 +117,7 @@ getfile(void) return; } get_line(); - if (flags & NOARGS && curlen < (size_t)owidth) + if (flags & NOARGS && curlen < owidth) flags |= ONEPERLINE; if (flags & ONEPERLINE) icols = 1; @@ -226,7 +223,7 @@ prepfile(void) else if (orows == 0 && ocols == 0) { /* decide rows and cols */ ocols = owidth / colw; if (ocols == 0) { - warnx("display width %d is less than column width %zu", + warnx("display width %zu is less than column width %zu", owidth, colw); ocols = 1; } @@ -338,101 +335,95 @@ static void getargs(int ac, char *av[]) { long val; - char *p; + int ch; if (ac == 1) { flags |= NOARGS | TRANSPOSE; } - while (--ac && **++av == '-') - for (p = *av+1; *p; p++) - switch (*p) { - case 'T': - flags |= MTRANSPOSE; - /* FALLTHROUGH */ - case 't': - flags |= TRANSPOSE; - break; - case 'c': /* input col. separator */ - flags |= ONEISEPONLY; - /* FALLTHROUGH */ - case 's': /* one or more allowed */ - if (p[1]) - isep = *++p; - else - isep = '\t'; /* default is ^I */ - break; - case 'C': - flags |= ONEOSEPONLY; - /* FALLTHROUGH */ - case 'S': - if (p[1]) - osep = *++p; - else - osep = '\t'; /* default is ^I */ - break; - case 'w': /* window width, default 80 */ - p = getnum(&owidth, p, 0); - if (owidth <= 0) - errx(1, "width must be a positive integer"); - break; - case 'K': /* skip N lines */ - flags |= SKIPPRINT; - /* FALLTHROUGH */ - case 'k': /* skip, do not print */ - p = getnum(&skip, p, 0); - if (!skip) - skip = 1; - break; - case 'm': - flags |= NOTRIMENDCOL; - break; - case 'g': /* gutter space */ - p = getnum(&gutter, p, 0); - break; - case 'G': - p = getnum(&propgutter, p, 0); - break; - case 'e': /* each line is an entry */ - flags |= ONEPERLINE; - break; - case 'E': - flags |= ONEPERCHAR; - break; - case 'j': /* right adjust */ - flags |= RIGHTADJUST; - break; - case 'n': /* null padding for missing values */ - flags |= NULLPAD; - break; - case 'y': - flags |= RECYCLE; - break; - case 'H': /* print shape only */ - flags |= DETAILSHAPE; - /* FALLTHROUGH */ - case 'h': - flags |= SHAPEONLY; - break; - case 'z': /* squeeze col width */ - flags |= SQUEEZE; - break; - /*case 'p': - ipagespace = atoi(++p); (default is 1) - break;*/ - case 'o': /* col order */ - p = getlist(&cord, p); - break; - case 'b': - flags |= ICOLBOUNDS; - p = getlist(&icbd, p); - break; - case 'B': - flags |= OCOLBOUNDS; - p = getlist(&ocbd, p); - break; - default: - usage(); - } + + while ((ch = getopt(ac, av, "C::EG:HK:S::Tc::eg:hjk:mns::tw:yz")) != -1) + switch (ch) { + case 'T': + flags |= MTRANSPOSE; + /* FALLTHROUGH */ + case 't': + flags |= TRANSPOSE; + break; + case 'c': /* input col. separator */ + flags |= ONEISEPONLY; + /* FALLTHROUGH */ + case 's': /* one or more allowed */ + if (optarg != NULL) + isep = *optarg; + else + isep = '\t'; /* default is ^I */ + break; + case 'C': + flags |= ONEOSEPONLY; + /* FALLTHROUGH */ + case 'S': + if (optarg != NULL) + osep = *optarg; + else + osep = '\t'; /* default is ^I */ + break; + case 'w': /* window width, default 80 */ + val = getnum(optarg); + if (val <= 0) + errx(1, "width must be a positive integer"); + owidth = val; + break; + case 'K': /* skip N lines */ + flags |= SKIPPRINT; + /* FALLTHROUGH */ + case 'k': /* skip, do not print */ + skip = getnum(optarg); + if (skip < 1) + skip = 1; + break; + case 'm': + flags |= NOTRIMENDCOL; + break; + case 'g': /* gutter space */ + gutter = getnum(optarg); + break; + case 'G': + propgutter = getnum(optarg); + break; + case 'e': /* each line is an entry */ + flags |= ONEPERLINE; + break; + case 'E': + flags |= ONEPERCHAR; + break; + case 'j': /* right adjust */ + flags |= RIGHTADJUST; + break; + case 'n': /* null padding for missing values */ + flags |= NULLPAD; + break; + case 'y': + flags |= RECYCLE; + break; + case 'H': /* print shape only */ + flags |= DETAILSHAPE; + /* FALLTHROUGH */ + case 'h': + flags |= SHAPEONLY; + break; + case 'z': /* squeeze col width */ + flags |= SQUEEZE; + break; + /*case 'p': + ipagespace = atoi(optarg); (default is 1) + break;*/ + default: + usage(); + } + + av += optind; + ac -= optind; + /*if (!osep) osep = isep;*/ switch (ac) { @@ -458,56 +449,14 @@ getargs(int ac, char *av[]) } } -static char * -getlist(short **list, char *p) +static long +getnum(const char *p) { - int count = 1; - char *t; + char *ep; + long val; - for (t = p + 1; *t; t++) { - if (!isdigit((unsigned char)*t)) - errx(1, - "option %.1s requires a list of unsigned numbers separated by commas", t); - count++; - while (*t && isdigit((unsigned char)*t)) - t++; - if (*t != ',') - break; - } - if (!(*list = (short *) malloc(count * sizeof(short)))) - errx(1, "no list space"); - count = 0; - for (t = p + 1; *t; t++) { - (*list)[count++] = atoi(t); - printf("++ %d ", (*list)[count-1]); - fflush(stdout); - while (*t && isdigit((unsigned char)*t)) - t++; - if (*t != ',') - break; - } - (*list)[count] = 0; - return(t - 1); -} - -/* - * num = number p points to; if (strict) complain - * returns pointer to end of num - */ -static char * -getnum(int *num, char *p, int strict) -{ - char *t = p; - - if (!isdigit((unsigned char)*++t)) { - if (strict || *t == '-' || *t == '+') - errx(1, "option %.1s requires an unsigned integer", p); - *num = 0; - return(p); - } - *num = atoi(t); - while (*++t) - if (!isdigit((unsigned char)*t)) - break; - return(--t); + val = strtol(p, &ep, 10); + if (*ep != '\0') + errx(1, "invalid integer %s", p); + return (val); } diff --git a/usr.bin/rs/tests/rs_test.sh b/usr.bin/rs/tests/rs_test.sh index 1960f9bb0019..1047a6557ae4 100644 --- a/usr.bin/rs/tests/rs_test.sh +++ b/usr.bin/rs/tests/rs_test.sh @@ -100,7 +100,7 @@ k_flag_head() k_flag_body() { - atf_check -s exit:0 -o empty rs -k < /dev/null + atf_check -s exit:0 -o empty rs -k 1 < /dev/null } atf_test_case K_flag @@ -112,7 +112,7 @@ K_flag_head() K_flag_body() { atf_check -s exit:0 -o inline:" -" rs -K < /dev/null +" rs -K 1 < /dev/null } atf_test_case g_flag @@ -123,7 +123,7 @@ g_flag_head() g_flag_body() { - atf_check -s exit:0 -o empty rs -g < /dev/null + atf_check -s exit:0 -o empty rs -g 1 < /dev/null } atf_test_case G_flag @@ -134,7 +134,7 @@ G_flag_head() G_flag_body() { - atf_check -s exit:0 -o empty rs -G < /dev/null + atf_check -s exit:0 -o empty rs -G 10 < /dev/null } atf_test_case e_flag @@ -237,7 +237,8 @@ invalid_usage_head() invalid_usage_body() { - atf_check -s not-exit:0 -e inline:"rs: width must be a positive integer + atf_check -s not-exit:0 -e inline:"rs: option requires an argument -- w +usage: rs [-[csCS][x][kKgGw][N]tTeEnyjhHmz] [rows [cols]] " rs -w }