~sircmpwn/ctools

echo: Improve POSIX compliance v3 PROPOSED

Haelwenn (lanodan) Monnier
Haelwenn (lanodan) Monnier: 1
 echo: Improve POSIX compliance

 3 files changed, 37 insertions(+), 116 deletions(-)
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~sircmpwn/ctools/patches/9661/mbox | git am -3
Learn more about email & git
View this thread in the archives

[PATCH v3] echo: Improve POSIX compliance Export this patch

Haelwenn (lanodan) Monnier
* `--` is to be treated as a string so getopt() can’t be used
* -n is implementation-defined, treating it as a string as there is no options
* Remove XSI support, simplifying the whole thing
* Buffer arguments to write(2) then only once
---
 doc/echo.1.scd | 35 ++------------------
 src/echo.c     | 90 ++++++++------------------------------------------
 test/echo      | 28 +++++++++++-----
 3 files changed, 37 insertions(+), 116 deletions(-)

diff --git a/doc/echo.1.scd b/doc/echo.1.scd
index 0ddc7e6..9a9c43b 100644
--- a/doc/echo.1.scd
+++ b/doc/echo.1.scd
@@ -6,7 +6,7 @@ echo - writes arguments to the standard output

# SYNOPSIS

*echo* [-n] [_string_...]
*echo* [_string_...]

# DESCRIPTION

@@ -17,41 +17,12 @@ is still printed.

# OPTIONS

*-n*
	Supress the output of a final newline after the last argument
	None. -- as well as -n are treated as a string operand.

# OPERANDS

	_string_
		string to be written to standard output. If -n is used as argument 
		the following escape characters will be recognized:

		\\a
			<alert>
		
		\\b
			<backspace>
		
		\\c
			Suppress the final <newline> and all characters following
		
		\\f
			<form-feed>

		\\n
			<newline>

		\\r
			<carrieage-return>
		
		\\t
			<tab>
		
		\\v 
			<vertical-tab>
		
		\\\\
			<backslash>
		string to be written to standard output.

# DISCLAIMER

diff --git a/src/echo.c b/src/echo.c
index fcd33df..e5a05d2 100644
--- a/src/echo.c
+++ b/src/echo.c
@@ -1,87 +1,25 @@
#include <getopt.h>
#include <stdio.h>
#include <stdlib.h>
#include <limits.h>
#include <string.h>
#include <unistd.h>

static void
usage(void)
{
	fprintf(stderr, "usage: echo [-n] [string...]");
}

int
main(int argc, char *argv[])
{
	int nflag = 0;
	char opt;
	// Use ARG_MAX assuming it counts one character between each argument
	char arguments[ARG_MAX];
	size_t freesize = ARG_MAX;

	while ((opt = getopt(argc, argv, "n")) != -1) {
		switch (opt) {
		case 'n':
			nflag = 1;
			break;
		default:
			usage();
			return 1;
		}
	}
	strncat(arguments, argv[1], freesize);
	freesize -= strlen(argv[1]);

	for (int i = optind; i < argc; ++i) {
		char *arg = NULL;
		for (arg = argv[i]; *arg; arg++) {
			if (arg[0] == '\\' && nflag) {
				switch (arg[1]) {
				case 'a':
					putchar('\a');
					arg++;
					break;
				case 'b':
					putchar('\b');
					arg++;
					break;
				case 'c':
					putchar('\n');
					arg++;
					return 0;
				case 'f':
					putchar('\f');
					arg++;
					break;
				case 'n':
					putchar('\n');
					arg++;
					break;
				case 'r':
					putchar('\r');
					arg++;
					break;
				case 't':
					putchar('\t');
					arg++;
					break;
				case 'v':
					putchar('\v');
					arg++;
					break;
				case '\\':
					putchar('\\');
					arg++;
					break;
				default:
					putchar(arg[0]);
					break;
				}
			} else {
				putchar(arg[0]);
			}
		}
		if (i != argc - 1) {
			putchar(' ');
		}
	for(int i = 2; i < argc && freesize > 1; i++) {
		strcat(arguments, " ");
		freesize--;
		strncat(arguments, argv[i], freesize - 1);
		freesize -= strlen(argv[i]);
	}

	if (!nflag) {
		putchar('\n');
	}
	printf("%s\n", arguments);

	return 0;
}
diff --git a/test/echo b/test/echo
index 3004665..47460c6 100644
--- a/test/echo
+++ b/test/echo
@@ -12,18 +12,30 @@ should_handle_two_string() (
	[ "$ct" = "this is a test string this second string" ]
)

should_handle_tab_char() (
	ct="$(../build/echo -n "\tthis is a test string")"
	[ "$ct" = '	this is a test string' ]
should_not_handle_tab_char() (
	ct="$(../build/echo "\tthis is a test string")"
	[ "$ct" = '\tthis is a test string' ]
)

should_handle_c_escape_char() (
	ct="$(../build/echo -n "this is a \ctest string")"
	[ "$ct" = "this is a " ]
should_not_handle_c_escape_char() (
	ct="$(../build/echo "this is a \ctest string")"
	[ "$ct" = "this is a \ctest string" ]
)

should_not_handle_double_dash() {
	ct="$(./echo -- "this is a test string")"
	[ "$ct" = "-- this is a test string" ]
}

should_not_handle_dash_n() {
	ct="$(./echo -n "this is a test string")"
	[ "$ct" = "-n this is a test string" ]
}

runtests \
	should_handle_one_string \
	should_handle_two_string \
	should_handle_tab_char \
	should_handle_c_escape_char
	should_not_handle_tab_char \
	should_not_handle_c_escape_char \
	should_not_handle_double_dash \
	should_not_handle_dash_n
-- 
2.24.1
> # OPTIONS
> 
> -*-n*
> - Supress the output of a final newline after the last argument
> + None. -- as well as -n are treated as a string operand.
It would be worth going into a little bit more detail here about the
rationale and such, because this fails the principle of least surprise.
> * `--` is to be treated as a string so getopt() can’t be used
> * -n is implementation-defined, treating it as a string as there is no
> options
> * Remove XSI support, simplifying the whole thing
> * Buffer arguments to write(2) then only once
> ---
> doc/echo.1.scd | 35 ++------------------
> src/echo.c | 90 ++++++++------------------------------------------
> test/echo | 28 +++++++++++-----
> 3 files changed, 37 insertions(+), 116 deletions(-)
> 
> diff --git a/doc/echo.1.scd b/doc/echo.1.scd
> index 0ddc7e6..9a9c43b 100644
> --- a/doc/echo.1.scd
> +++ b/doc/echo.1.scd
> @@ -6,7 +6,7 @@ echo - writes arguments to the standard output
> 
> # SYNOPSIS
> 
> -*echo* [-n] [_string_...]
> +*echo* [_string_...]
> 
> # DESCRIPTION
> 
> @@ -17,41 +17,12 @@ is still printed.
> 
> # OPTIONS
> 
> -*-n*
> - Supress the output of a final newline after the last argument
> + None. -- as well as -n are treated as a string operand.
> 
> # OPERANDS
> 
> _string_
> - string to be written to standard output. If -n is used as argument
> - the following escape characters will be recognized:
> -
> - \\a
> - <alert>
> -
> - \\b
> - <backspace>
> -
> - \\c
> - Suppress the final <newline> and all characters following
> -
> - \\f
> - <form-feed>
> -
> - \\n
> - <newline>
> -
> - \\r
> - <carrieage-return>
> -
> - \\t
> - <tab>
> -
> - \\v
> - <vertical-tab>
> -
> - \\\\
> - <backslash>
> + string to be written to standard output.
> 
> # DISCLAIMER
> 
> diff --git a/src/echo.c b/src/echo.c
> index fcd33df..e5a05d2 100644
> --- a/src/echo.c
> +++ b/src/echo.c
> @@ -1,87 +1,25 @@
> -#include <getopt.h>
> #include <stdio.h>
> -#include <stdlib.h>
> +#include <limits.h>
> #include <string.h>
> -#include <unistd.h>
> -
> -static void
> -usage(void)
> -{
> - fprintf(stderr, "usage: echo [-n] [string...]");
> -}
> 
> int
> main(int argc, char *argv[])
> {
> - int nflag = 0;
> - char opt;
> + // Use ARG_MAX assuming it counts one character between each argument
> + char arguments[ARG_MAX];
> + size_t freesize = ARG_MAX;
> 
> - while ((opt = getopt(argc, argv, "n")) != -1) {
> - switch (opt) {
> - case 'n':
> - nflag = 1;
> - break;
> - default:
> - usage();
> - return 1;
> - }
> - }
> + strncat(arguments, argv[1], freesize);
> + freesize -= strlen(argv[1]);
> 
> - for (int i = optind; i < argc; ++i) {
> - char *arg = NULL;
> - for (arg = argv[i]; *arg; arg++) {
> - if (arg[0] == '\\' && nflag) {
> - switch (arg[1]) {
> - case 'a':
> - putchar('\a');
> - arg++;
> - break;
> - case 'b':
> - putchar('\b');
> - arg++;
> - break;
> - case 'c':
> - putchar('\n');
> - arg++;
> - return 0;
> - case 'f':
> - putchar('\f');
> - arg++;
> - break;
> - case 'n':
> - putchar('\n');
> - arg++;
> - break;
> - case 'r':
> - putchar('\r');
> - arg++;
> - break;
> - case 't':
> - putchar('\t');
> - arg++;
> - break;
> - case 'v':
> - putchar('\v');
> - arg++;
> - break;
> - case '\\':
> - putchar('\\');
> - arg++;
> - break;
> - default:
> - putchar(arg[0]);
> - break;
> - }
> - } else {
> - putchar(arg[0]);
> - }
> - }
> - if (i != argc - 1) {
> - putchar(' ');
> - }
> + for(int i = 2; i < argc && freesize > 1; i++) {
> + strcat(arguments, " ");
Sorry, sent email too early. Need space after for, and let's call it
args (arguments is too verbose). Also should always use strncat to avoid
buffer overflows.