[PATCH 00/13]: text-utils fixes

[PATCH 00/13]: text-utils fixes

[Sami Kerola]

Three first patches are warnign removals.

The col and colctr fixes are are mostly about getting --help &
--version to work, plus some coding style fixes, argument
checking etc trivial stuff.

Patch 9 is unusual. It removes all goto statements from colrm
which, I hope, makes the code more understandable. Due the
unusuality of the patch I made basic regression test.

All patches are available from github in branch text-utils.
git://github.com/kerolasa/lelux-utiliteetit.git

[PATCH 08/13] col: check with strtol_or_err option argument

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 text-utils/Makefile.am |    2 ++
 text-utils/col.c       |    8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/text-utils/Makefile.am b/text-utils/Makefile.am
index 513f1c3..8700889 100644
--- a/text-utils/Makefile.am
+++ b/text-utils/Makefile.am
@@ -4,6 +4,8 @@ EXTRA_DIST = README.clear README.col
 
 usrbin_exec_PROGRAMS = col colcrt colrm column hexdump rev line tailf
 
+col_SOURCES = col.c $(top_srcdir)/lib/strutils.c
+
 hexdump_SOURCES = hexdump.c conv.c display.c hexsyntax.c parse.c \
 		  hexdump.h $(top_srcdir)/lib/strutils.c
 
diff --git a/text-utils/col.c b/text-utils/col.c
index 0740687..cdf0036 100644
--- a/text-utils/col.c
+++ b/text-utils/col.c
@@ -55,6 +55,7 @@
 #include "nls.h"
 #include "xalloc.h"
 #include "widechar.h"
+#include "strutils.h"
 
 #define	BS	'\b'		/* backspace */
 #define	TAB	'\t'		/* tab */
@@ -155,6 +156,7 @@ int main(int argc, char **argv)
 	int this_line;			/* line l points to */
 	int nflushd_lines;		/* number of lines that were flushed */
 	int adjust, opt, warned;
+	unsigned long tmplong;
 	int ret = EXIT_SUCCESS;
 
 	static const struct option longopts[] = {
@@ -189,8 +191,10 @@ int main(int argc, char **argv)
 			compress_spaces = 1;
 			break;
 		case 'l':		/* buffered line count */
-			if ((max_bufd_lines = atoi(optarg)) <= 0)
-				errx(EXIT_FAILURE, _("bad -l argument %s."), optarg);
+			tmplong = strtoul_or_err(optarg, _("bad -l argument"));
+			if ((INT_MAX / 2) < tmplong)
+				errx(EXIT_FAILURE, _("argument %lu is too large"), tmplong);
+			max_bufd_lines = (int) tmplong;
 			break;
 		case 'p':
 			pass_unknown_seqs = 1;
-- 
1.7.5.2

[PATCH 09/13] colrm: gotos, long options and argument checking

[Sami Kerola]

Unnecessary goto jumps where replaced by a simple function, and
two loops. Long options support help and version, and the two
arguments this command has are now validated with strtoul_or_err.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 text-utils/Makefile.am |    2 +
 text-utils/colrm.c     |  143 ++++++++++++++++++++++++++++++++---------------
 2 files changed, 99 insertions(+), 46 deletions(-)

diff --git a/text-utils/Makefile.am b/text-utils/Makefile.am
index 8700889..0255033 100644
--- a/text-utils/Makefile.am
+++ b/text-utils/Makefile.am
@@ -6,6 +6,8 @@ usrbin_exec_PROGRAMS = col colcrt colrm column hexdump rev line tailf
 
 col_SOURCES = col.c $(top_srcdir)/lib/strutils.c
 
+colrm_SOURCES = colrm.c $(top_srcdir)/lib/strutils.c
+
 hexdump_SOURCES = hexdump.c conv.c display.c hexsyntax.c parse.c \
 		  hexdump.h $(top_srcdir)/lib/strutils.c
 
diff --git a/text-utils/colrm.c b/text-utils/colrm.c
index e8b1ea4..fa56f2d 100644
--- a/text-utils/colrm.c
+++ b/text-utils/colrm.c
@@ -40,67 +40,80 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <getopt.h>
 
 #include "nls.h"
 #include "widechar.h"
+#include "strutils.h"
+#include "c.h"
 
 /*
 COLRM removes unwanted columns from a file
 	Jeff Schriebman  UC Berkeley 11-74
 */
 
-int
-main(int argc, char **argv)
+static void __attribute__ ((__noreturn__)) usage(FILE * out)
 {
-	register int ct, first, last;
-	register wint_t c;
-	int i, w;
+	fprintf(out, _("\nUsage:\n"
+		       " %s [startcol [endcol]]\n"),
+		       program_invocation_short_name);
+
+	fprintf(out, _("\nOptions:\n"
+		       " -V, --version   output version information and exit\n"
+		       " -h, --help      display this help and exit\n\n"));
+
+	fprintf(out, _("%s reads from standard input and writes to standard output\n\n"),
+		       program_invocation_short_name);
+
+	exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
+}
+
+int process_input(unsigned long first, unsigned long last)
+{
+	unsigned long ct = 0;
+	wint_t c;
+	unsigned long i;
+	int w;
 	int padding;
 
-	setlocale(LC_ALL, "");
+	for (;;) {
+		c = getwc(stdin);
+		if (c == WEOF)
+			return 0;
+		if (c == '\t')
+			w = ((ct + 8) & ~7) - ct;
+		else if (c == '\b')
+			w = (ct ? ct - 1 : 0) - ct;
+		else {
+			w = wcwidth(c);
+			if (w < 0)
+				w = 0;
+		}
+		ct += w;
+		if (c == '\n') {
+			putwc(c, stdout);
+			ct = 0;
+			continue;
 
-	first = 0;
-	last = 0;
-	if (argc > 1)
-		first = atoi(*++argv);
-	if (argc > 2)
-		last = atoi(*++argv);
-
-start:
-	ct = 0;
-loop1:
-	c = getwc(stdin);
-	if (c == WEOF)
-		goto fin;
-	if (c == '\t')
-		w = ((ct + 8) & ~7) - ct;
-	else if (c == '\b')
-		w = (ct ? ct - 1 : 0) - ct;
-	else {
-		w = wcwidth(c);
-		if (w < 0)
-			w = 0;
-	}
-	ct += w;
-	if (c == '\n') {
-		putwc(c, stdout);
-		goto start;
-	}
-	if (!first || ct < first) {
-		putwc(c, stdout);
-		goto loop1;
+		}
+		if (!first || ct < first) {
+			putwc(c, stdout);
+			continue;
+		}
+		break;
 	}
-	for (i = ct-w+1; i < first; i++)
+
+	for (i = ct - w + 1; i < first; i++)
 		putwc(' ', stdout);
 
-/* Loop getting rid of characters */
+	/* Loop getting rid of characters */
 	while (!last || ct < last) {
 		c = getwc(stdin);
 		if (c == WEOF)
-			goto fin;
+			return 0;
 		if (c == '\n') {
 			putwc(c, stdout);
-			goto start;
+			return 1;
 		}
 		if (c == '\t')
 			ct = (ct + 8) & ~7;
@@ -116,25 +129,63 @@ loop1:
 
 	padding = 0;
 
-/* Output last of the line */
+	/* Output last of the line */
 	for (;;) {
 		c = getwc(stdin);
 		if (c == WEOF)
 			break;
 		if (c == '\n') {
 			putwc(c, stdout);
-			goto start;
+			return 1;
 		}
 		if (padding == 0 && last < ct) {
-			for (i = last; i <ct; i++)
+			for (i = last; i < ct; i++)
 				putwc(' ', stdout);
 			padding = 1;
 		}
 		putwc(c, stdout);
 	}
-fin:
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	unsigned long first = 0, last = 0;
+	int opt;
+
+	static const struct option longopts[] = {
+		{"version", no_argument, 0, 'V'},
+		{"help", no_argument, 0, 'h'},
+		{NULL, 0, 0, 0}
+	};
+
+	setlocale(LC_ALL, "");
+
+	while ((opt =
+		getopt_long(argc, argv, "bfhl:pxVH", longopts,
+			    NULL)) != -1)
+		switch (opt) {
+		case 'V':
+			printf(_("%s from %s\n"),
+			       program_invocation_short_name,
+			       PACKAGE_STRING);
+			return EXIT_SUCCESS;
+		case 'h':
+			usage(stdout);
+		default:
+			usage(stderr);
+		}
+
+	if (argc > 1)
+		first = strtoul_or_err(*++argv, _("first argument"));
+	if (argc > 2)
+		last = strtoul_or_err(*++argv, _("second argument"));
+
+	while (process_input(first, last))
+		;
+
 	fflush(stdout);
 	if (ferror(stdout) || fclose(stdout))
-		return 1;
-	return 0;
+		return EXIT_FAILURE;
+	return EXIT_SUCCESS;
 }
-- 
1.7.5.2

[PATCH 10/13] docs: colrm manual update

[Sami Kerola]

Inform about long, help & version and options.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 text-utils/colrm.1 |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/text-utils/colrm.1 b/text-utils/colrm.1
index 1f3d4b0..04a41b1 100644
--- a/text-utils/colrm.1
+++ b/text-utils/colrm.1
@@ -51,6 +51,14 @@ If called with two parameters the columns from the first column
 to the last column will be removed.
 .Pp
 Column numbering starts with column 1.
+.Pp
+The options are as follows:
+.Bl -tag -width "-lnum"
+.It Fl V, Fl Fl version
+Output version information and exit.
+.It Fl H, Fl Fl help
+Output help and exit.
+.El
 .Sh SEE ALSO
 .Xr awk 1 ,
 .Xr column 1 ,
-- 
1.7.5.2

Re: [PATCH 08/13] col: check with strtol_or_err option argument

[Karel Zak]

On Tue, May 24, 2011 at 10:56:39PM +0200, Sami Kerola wrote:
>  		case 'l':		/* buffered line count */
> -			if ((max_bufd_lines = atoi(optarg)) <= 0)
> -				errx(EXIT_FAILURE, _("bad -l argument %s."), optarg);
> +			tmplong = strtoul_or_err(optarg, _("bad -l argument"));
> +			if ((INT_MAX / 2) < tmplong)

 Why we need this magical (INT_MAX / 2) limit?

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 09/13] colrm: gotos, long options and argument checking

[Karel Zak]

On Tue, May 24, 2011 at 10:56:40PM +0200, Sami Kerola wrote:
> +int main(int argc, char **argv)
> +{
> +	unsigned long first = 0, last = 0;
> +	int opt;
> +
> +	static const struct option longopts[] = {
> +		{"version", no_argument, 0, 'V'},
> +		{"help", no_argument, 0, 'h'},
> +		{NULL, 0, 0, 0}
> +	};
> +
> +	setlocale(LC_ALL, "");
 
and

    bindtextdomain(PACKAGE, LOCALEDIR);
    textdomain(PACKAGE);

right?

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Fwd: [PATCH 00/13]: text-utils fixes

[Sami Kerola]

On Wed, Jun 1, 2011 at 10:10, Karel Zak <kzak@redhat.com> wrote:
>> All patches are available from github in branch text-utils.
>> git://github.com/kerolasa/lelux-utiliteetit.git
>
>  All applied, except patches 8, 9 and 10.

I took a look of the rejected patches.

[PATCH 08/13] col
> Why we need this magical (INT_MAX / 2) limit?

The col command is counting stuff in half lines i.e. the counts are
double what one would intuitively think. I wrote the code segment
different way, and commented it. Perhaps the lines are more
understandable now.

[PATCH 09/13] colrm
> and
> bindtextdomain(PACKAGE, LOCALEDIR);
> textdomain(PACKAGE);
> right?

Wilco.

I amend the fixes, and pushed with force to repository.

-- 
   Sami Kerola
   http://www.iki.fi/kerolasa/

Re: Fwd: [PATCH 00/13]: text-utils fixes

[Karel Zak]

On Sun, Jun 05, 2011 at 08:47:48PM +0200, Sami Kerola wrote:
> I amend the fixes, and pushed with force to repository.

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com