DASH Shell discussions
 help / color / mirror / Atom feed
* [PATCH] echo: fix octal escaping with \1...\7
@ 2011-10-25 21:58 Mike Frysinger
  2011-10-26  7:59 ` Stephane CHAZELAS
  2011-10-31  3:41 ` Herbert Xu
  0 siblings, 2 replies; 11+ messages in thread
From: Mike Frysinger @ 2011-10-25 21:58 UTC (permalink / raw)
  To: dash; +Cc: Herbert Xu

POSIX states that octal escape sequences should take the form \0num
when using echo.  dash however additionally treats \num as an octal
sequence.  This breaks some packages (like libtool) who attempt to
use strings with these escape sequences via variables to execute sed
(since sed ends up getting passed a byte instead of a literal \1).

The code that consumes this sequence includes a comment that indicates
it doesn't actually mean to do this.  So simplify the code a bit by
ignoring these sequences that lack a leading 0 and falling through to
the existing escape parsing logic.

before:
	$ echo '\1' | hexdump -C
	00000000  01 0a                 |..|
after:
	$ echo '\1' | hexdump -C
	00000000  5c 31 0a              |\1.|
(existing \01 sequence still works the same)

This also slightly shrinks the resulting compiled code :).

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
Note: I assume this still applies to the latest git.  The last
	checkout I have is from Sep 2010 though, and kernel.org does
	not yet have the dash tree back on it.

 src/bltin/printf.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/src/bltin/printf.c b/src/bltin/printf.c
index b0c3774..1d373f9 100644
--- a/src/bltin/printf.c
+++ b/src/bltin/printf.c
@@ -247,18 +247,10 @@ conv_escape_str(char *str)
 		 * They start with a \0, and are followed by 0, 1, 2, 
 		 * or 3 octal digits. 
 		 */
-		if (ch == '0') {
-			unsigned char i;
-			i = 3;
-			ch = 0;
-			do {
-				unsigned k = octtobin(*str);
-				if (k > 7)
-					break;
-				str++;
-				ch <<= 3;
-				ch += k;
-			} while (--i);
+		if (ch >= '1' && ch <= '9') {
+			/* Filter \1...\9; let \0 fall to conv_escape(). */
+			ch = '\\';
+			--str;
 			continue;
 		}
 
-- 
1.7.6.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-25 21:58 [PATCH] echo: fix octal escaping with \1...\7 Mike Frysinger
@ 2011-10-26  7:59 ` Stephane CHAZELAS
  2011-10-31  3:41 ` Herbert Xu
  1 sibling, 0 replies; 11+ messages in thread
From: Stephane CHAZELAS @ 2011-10-26  7:59 UTC (permalink / raw)
  To: dash

2011-10-25, 17:58(-04), Mike Frysinger:
> POSIX states that octal escape sequences should take the form \0num
> when using echo.

Only with the XSI option (Unix), for POSIX, echo "\whatever" is
unspecified. But as far as I can tell even with XSI, "echo
'\123'" is unspecified as well, so dash is free to do what it
likes here.

> dash however additionally treats \num as an octal
> sequence.  This breaks some packages (like libtool) who attempt to
> use strings with these escape sequences via variables to execute sed
> (since sed ends up getting passed a byte instead of a literal \1).

Given that the result of  

echo "\123" is unspecified, those scripts would not be POSIX and
they are those to be fixed.

-- 
Stephane


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-25 21:58 [PATCH] echo: fix octal escaping with \1...\7 Mike Frysinger
  2011-10-26  7:59 ` Stephane CHAZELAS
@ 2011-10-31  3:41 ` Herbert Xu
  2011-10-31  4:23   ` Mike Frysinger
  1 sibling, 1 reply; 11+ messages in thread
From: Herbert Xu @ 2011-10-31  3:41 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: dash

Mike Frysinger <vapier@gentoo.org> wrote:
> POSIX states that octal escape sequences should take the form \0num
> when using echo.  dash however additionally treats \num as an octal
> sequence.  This breaks some packages (like libtool) who attempt to
> use strings with these escape sequences via variables to execute sed
> (since sed ends up getting passed a byte instead of a literal \1).

OK this is a bit of problem.  From our conversation I had the
impression that you were referring to the lack of support of
escape codes, rather than unwanted support.

If it was the former I could easily add it if POSIX said so,
however, as this is an existing feature there may well be scripts
out there that depend on it.  So removing it is not an option
unless it is explicitly forbidden by POSIX.

In any case, scripts that rely on escape codes like this are
simply broken and should either be fixed to use printf or just
run with #!/bin/bash.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-31  3:41 ` Herbert Xu
@ 2011-10-31  4:23   ` Mike Frysinger
  2011-10-31 13:12     ` Eric Blake
  0 siblings, 1 reply; 11+ messages in thread
From: Mike Frysinger @ 2011-10-31  4:23 UTC (permalink / raw)
  To: Herbert Xu; +Cc: dash

[-- Attachment #1: Type: Text/Plain, Size: 1975 bytes --]

On Sunday 30 October 2011 23:41:58 Herbert Xu wrote:
> Mike Frysinger wrote:
> > POSIX states that octal escape sequences should take the form \0num
> > when using echo.  dash however additionally treats \num as an octal
> > sequence.  This breaks some packages (like libtool) who attempt to
> > use strings with these escape sequences via variables to execute sed
> > (since sed ends up getting passed a byte instead of a literal \1).
> 
> OK this is a bit of problem.  From our conversation I had the
> impression that you were referring to the lack of support of
> escape codes, rather than unwanted support.
> 
> If it was the former I could easily add it if POSIX said so,
> however, as this is an existing feature there may well be scripts
> out there that depend on it.  So removing it is not an option
> unless it is explicitly forbidden by POSIX.

i'm not seeing how this jives with dash's goal.  if it intends to be a 
fast/small POSIX compliant shell while punting (almost) all the rest, then why 
carry additional functionality that POSIX doesn't even mention in passing ?  
this isn't "documented but optional extended functionality", but rather the 
realm of "anything goes".  otherwise we approach the same realm that dash was 
created to avoid -- carrying lots of cruft that slow things down because 
scripts use it rather than POSIX mandating it.

as a comparison, bash/ksh/tcsh/zsh/busybox[ash] all behave the way my patch 
updates dash to operate ... i would test more shells, but these tend to be the 
standards that everyone compares against.  i can't see people writing scripts 
that only work under dash either.

> In any case, scripts that rely on escape codes like this are
> simply broken and should either be fixed to use printf or just
> run with #!/bin/bash.

they're relying on these escape codes not being interpreted as escape codes 
(which every other shell appears to do), not the other way around
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-31  4:23   ` Mike Frysinger
@ 2011-10-31 13:12     ` Eric Blake
  2011-10-31 13:35       ` Paul Gilmartin
                         ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Eric Blake @ 2011-10-31 13:12 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: Herbert Xu, dash, bug-libtool

[adding bug-libtool]

On 10/30/2011 10:23 PM, Mike Frysinger wrote:
> On Sunday 30 October 2011 23:41:58 Herbert Xu wrote:
>> Mike Frysinger wrote:
>>> POSIX states that octal escape sequences should take the form \0num
>>> when using echo.  dash however additionally treats \num as an octal
>>> sequence.  This breaks some packages (like libtool) who attempt to
>>> use strings with these escape sequences via variables to execute sed
>>> (since sed ends up getting passed a byte instead of a literal \1).

That's a bug in libtool for using "echo '\1'" and expecting sane 
behavior.  Can you provide more details on this libtool bug, so we can 
get it fixed in libtool?  Or perhaps it has already been fixed in modern 
libtool, and you are just encountering it in an older version?

>>
>> OK this is a bit of problem.  From our conversation I had the
>> impression that you were referring to the lack of support of
>> escape codes, rather than unwanted support.
>>
>> If it was the former I could easily add it if POSIX said so,
>> however, as this is an existing feature there may well be scripts
>> out there that depend on it.  So removing it is not an option
>> unless it is explicitly forbidden by POSIX.
>
> i'm not seeing how this jives with dash's goal.  if it intends to be a
> fast/small POSIX compliant shell while punting (almost) all the rest, then why
> carry additional functionality that POSIX doesn't even mention in passing ?
> this isn't "documented but optional extended functionality", but rather the
> realm of "anything goes".  otherwise we approach the same realm that dash was
> created to avoid -- carrying lots of cruft that slow things down because
> scripts use it rather than POSIX mandating it.
>
> as a comparison, bash/ksh/tcsh/zsh/busybox[ash] all behave the way my patch
> updates dash to operate ... i would test more shells, but these tend to be the
> standards that everyone compares against.  i can't see people writing scripts
> that only work under dash either.
>
>> In any case, scripts that rely on escape codes like this are
>> simply broken and should either be fixed to use printf or just
>> run with #!/bin/bash.
>
> they're relying on these escape codes not being interpreted as escape codes
> (which every other shell appears to do), not the other way around

Scripts that rely on a certain interpretation of "echo '\1'" are broken 
regardless of how dash behaves; but that said, since POSIX doesn't 
require dash's current behavior, and since the proposed patch makes dash 
both smaller and more like other shells in treating it as an extension 
that means a literal 1 rather than an octal escape, I would be in favor 
of making the change in dash.

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-31 13:12     ` Eric Blake
@ 2011-10-31 13:35       ` Paul Gilmartin
  2011-10-31 14:03         ` Eric Blake
  2011-10-31 14:56         ` Stephane CHAZELAS
  2011-10-31 18:07       ` Harald van Dijk
  2011-10-31 18:39       ` Mike Frysinger
  2 siblings, 2 replies; 11+ messages in thread
From: Paul Gilmartin @ 2011-10-31 13:35 UTC (permalink / raw)
  To: Eric Blake; +Cc: dash

On Oct 31, 2011, at 07:12, Eric Blake wrote:

> [adding bug-libtool]
>  
[removing, because I'm not registered.]

> On 10/30/2011 10:23 PM, Mike Frysinger wrote:
>> On Sunday 30 October 2011 23:41:58 Herbert Xu wrote:
>>> Mike Frysinger wrote:
>>>> POSIX states that octal escape sequences should take the form \0num
>>>> when using echo.  dash however additionally treats \num as an octal
>>>> sequence.  This breaks some packages (like libtool) who attempt to
>>>> use strings with these escape sequences via variables to execute sed
>>>> (since sed ends up getting passed a byte instead of a literal \1).
>  
> That's a bug in libtool for using "echo '\1'" and expecting sane behavior.  Can you provide more details on this libtool bug, so we can get it fixed in libtool?  Or perhaps it has already been fixed in modern libtool, and you are just encountering it in an older version?
>  
Yes, there's value in coding defensively.  However:

I used to know a statement in POSIX that builtins should behave
identically to the executables in /bin (or perhaps /usr/bin)
except for performance.  So, testing with dash on Ubuntu:

\! $ echo "\1"
\x01
\! $ /bin/echo "\1"
\1
\! $ 

I recognize that following this precept would make the behavior
of the builtin "echo" platform-dependent.  But that could be
managed in autoconf.

-- gil


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-31 13:35       ` Paul Gilmartin
@ 2011-10-31 14:03         ` Eric Blake
  2011-10-31 14:56         ` Stephane CHAZELAS
  1 sibling, 0 replies; 11+ messages in thread
From: Eric Blake @ 2011-10-31 14:03 UTC (permalink / raw)
  To: Paul Gilmartin; +Cc: dash

On 10/31/2011 07:35 AM, Paul Gilmartin wrote:
> I used to know a statement in POSIX that builtins should behave
> identically to the executables in /bin (or perhaps /usr/bin)
> except for performance.

Only in regards to standardized use of those utilities.  'echo "\1"' is 
not standardized, so it is allowed to differ between the dash built-in 
and /bin/echo.

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-31 13:35       ` Paul Gilmartin
  2011-10-31 14:03         ` Eric Blake
@ 2011-10-31 14:56         ` Stephane CHAZELAS
  1 sibling, 0 replies; 11+ messages in thread
From: Stephane CHAZELAS @ 2011-10-31 14:56 UTC (permalink / raw)
  To: dash

2011-10-31, 07:35(-06), Paul Gilmartin:
> On Oct 31, 2011, at 07:12, Eric Blake wrote:
>
>> [adding bug-libtool]
>>  
> [removing, because I'm not registered.]
>
>> On 10/30/2011 10:23 PM, Mike Frysinger wrote:
>>> On Sunday 30 October 2011 23:41:58 Herbert Xu wrote:
>>>> Mike Frysinger wrote:
>>>>> POSIX states that octal escape sequences should take the form \0num
>>>>> when using echo.  dash however additionally treats \num as an octal
>>>>> sequence.  This breaks some packages (like libtool) who attempt to
>>>>> use strings with these escape sequences via variables to execute sed
>>>>> (since sed ends up getting passed a byte instead of a literal \1).
>>  
>> That's a bug in libtool for using "echo '\1'" and expecting sane behavior.  Can you provide more details on this libtool bug, so we can get it fixed in libtool?  Or perhaps it has already been fixed in modern libtool, and you are just encountering it in an older version?
>>  
> Yes, there's value in coding defensively.  However:
>
> I used to know a statement in POSIX that builtins should behave
> identically to the executables in /bin (or perhaps /usr/bin)
> except for performance.  So, testing with dash on Ubuntu:
[...]

This can only reasonably be done on systems where the shell and
utilities are maintained by the same persons. On my system,
/bin/echo '\FS' starts a flight simulator. Should I ask dash to
implement that same flight simulator?

See also GNU test that doesn't behave the same as GNU bash's
"test" (test a \< b for instance).

-- 
Stephane


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-31 13:12     ` Eric Blake
  2011-10-31 13:35       ` Paul Gilmartin
@ 2011-10-31 18:07       ` Harald van Dijk
  2011-10-31 18:39       ` Mike Frysinger
  2 siblings, 0 replies; 11+ messages in thread
From: Harald van Dijk @ 2011-10-31 18:07 UTC (permalink / raw)
  To: dash

On Mon, 2011-10-31 at 07:12 -0600, Eric Blake wrote:
> Scripts that rely on a certain interpretation of "echo '\1'" are broken 
> regardless of how dash behaves; but that said, since POSIX doesn't 
> require dash's current behavior, and since the proposed patch makes dash 
> both smaller and more like other shells in treating it as an extension 
> that means a literal 1 rather than an octal escape, I would be in favor 
> of making the change in dash.

The problem with that is that the patch makes dash behave differently
from other shells in its interpretation of printf's %b format. bash, for
instance, does accept \1 escape sequences there, as an extension to
POSIX. Currently, so does dash. With this patch, it stops working in
dash. And unsharing the code between echo and printf would probably make
dash larger.

There's also another problem: the patch doesn't work as advertised
anyway. It should continue to make echo "\0101" print a single character
followed by newline. Instead, with that patch dash takes \010 as an
escape sequence, and then follows it by 1 and a newline.

Cheers,
Harald


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-31 13:12     ` Eric Blake
  2011-10-31 13:35       ` Paul Gilmartin
  2011-10-31 18:07       ` Harald van Dijk
@ 2011-10-31 18:39       ` Mike Frysinger
  2011-10-31 18:48         ` Harald van Dijk
  2 siblings, 1 reply; 11+ messages in thread
From: Mike Frysinger @ 2011-10-31 18:39 UTC (permalink / raw)
  To: Eric Blake; +Cc: Herbert Xu, dash, bug-libtool

[-- Attachment #1: Type: Text/Plain, Size: 3245 bytes --]

On Monday 31 October 2011 09:12:43 Eric Blake wrote:
> On 10/30/2011 10:23 PM, Mike Frysinger wrote:
> > On Sunday 30 October 2011 23:41:58 Herbert Xu wrote:
> >> Mike Frysinger wrote:
> >>> POSIX states that octal escape sequences should take the form \0num
> >>> when using echo.  dash however additionally treats \num as an octal
> >>> sequence.  This breaks some packages (like libtool) who attempt to
> >>> use strings with these escape sequences via variables to execute sed
> >>> (since sed ends up getting passed a byte instead of a literal \1).
> 
> That's a bug in libtool for using "echo '\1'" and expecting sane
> behavior.  Can you provide more details on this libtool bug, so we can
> get it fixed in libtool?  Or perhaps it has already been fixed in modern
> libtool, and you are just encountering it in an older version?

i plan on digging through the relevant packages and posting patches where 
applicable.  i might be wrong about the libtool side, but do know of at least 
one ax m4 file using it (which is what started this rat hole in the first 
place).  but i consider that a parallel issue :).

> >> OK this is a bit of problem.  From our conversation I had the
> >> impression that you were referring to the lack of support of
> >> escape codes, rather than unwanted support.
> >> 
> >> If it was the former I could easily add it if POSIX said so,
> >> however, as this is an existing feature there may well be scripts
> >> out there that depend on it.  So removing it is not an option
> >> unless it is explicitly forbidden by POSIX.
> > 
> > i'm not seeing how this jives with dash's goal.  if it intends to be a
> > fast/small POSIX compliant shell while punting (almost) all the rest,
> > then why carry additional functionality that POSIX doesn't even mention
> > in passing ? this isn't "documented but optional extended
> > functionality", but rather the realm of "anything goes".  otherwise we
> > approach the same realm that dash was created to avoid -- carrying lots
> > of cruft that slow things down because scripts use it rather than POSIX
> > mandating it.
> > 
> > as a comparison, bash/ksh/tcsh/zsh/busybox[ash] all behave the way my
> > patch updates dash to operate ... i would test more shells, but these
> > tend to be the standards that everyone compares against.  i can't see
> > people writing scripts that only work under dash either.
> > 
> >> In any case, scripts that rely on escape codes like this are
> >> simply broken and should either be fixed to use printf or just
> >> run with #!/bin/bash.
> > 
> > they're relying on these escape codes not being interpreted as escape
> > codes (which every other shell appears to do), not the other way around
> 
> Scripts that rely on a certain interpretation of "echo '\1'" are broken
> regardless of how dash behaves;

sure, i'm not arguing that logic

> but that said, since POSIX doesn't
> require dash's current behavior, and since the proposed patch makes dash
> both smaller and more like other shells in treating it as an extension
> that means a literal 1 rather than an octal escape, I would be in favor
> of making the change in dash.

right, that's what i'm going for
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] echo: fix octal escaping with \1...\7
  2011-10-31 18:39       ` Mike Frysinger
@ 2011-10-31 18:48         ` Harald van Dijk
  0 siblings, 0 replies; 11+ messages in thread
From: Harald van Dijk @ 2011-10-31 18:48 UTC (permalink / raw)
  To: dash

On Mon, 2011-10-31 at 14:39 -0400, Mike Frysinger wrote:
> i plan on digging through the relevant packages and posting patches where 
> applicable.  i might be wrong about the libtool side, but do know of at least 
> one ax m4 file using it (which is what started this rat hole in the first 
> place).  but i consider that a parallel issue :).

That one m4 file was ax_prefix_config_h.m4, for which I submitted a
patch upstream and which got accepted over a year ago:
<http://savannah.gnu.org/patch/index.php?7317>.

Cheers,
Harald


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2011-10-31 18:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-25 21:58 [PATCH] echo: fix octal escaping with \1...\7 Mike Frysinger
2011-10-26  7:59 ` Stephane CHAZELAS
2011-10-31  3:41 ` Herbert Xu
2011-10-31  4:23   ` Mike Frysinger
2011-10-31 13:12     ` Eric Blake
2011-10-31 13:35       ` Paul Gilmartin
2011-10-31 14:03         ` Eric Blake
2011-10-31 14:56         ` Stephane CHAZELAS
2011-10-31 18:07       ` Harald van Dijk
2011-10-31 18:39       ` Mike Frysinger
2011-10-31 18:48         ` Harald van Dijk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox