dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Harald van Dijk <harald@gigawatt.nl>
To: Herbert Xu <herbert@gondor.apana.org.au>, dash@vger.kernel.org
Cc: Stephane Chazelas <stephane.chazelas@gmail.com>,
	Gioele Barabucci <gioele@svario.it>
Subject: Re: [PATCH v3] builtin: Fix handling of trailing IFS white spaces
Date: Sun, 12 Jun 2016 12:35:15 +0200	[thread overview]
Message-ID: <575D3AE3.1080700@gigawatt.nl> (raw)
In-Reply-To: <20160607092539.GA30397@gondor.apana.org.au>

On 07/06/16 11:25, Herbert Xu wrote:
> If the very first character later gets zeroed, you'd be zeroing
> the character after the CTLESC, leaving the CTLESC in the value
> to be assigned to the last variable of read.

Ah, I see. Thanks for the explanation. While trying to make it misbehave 
I found that rmescapes removes trailing CTLESC, and readcmd_handle_line 
calls rmescapes before the trailing CTLESC gets a chance to cause 
problems. I was now still only able to see the problem when adding some 
extra debugging statements.

> If you got rid of the very first q=p assignment it should just work.

There is a later q=p assignment too that gets performed after CTLESC has 
been skipped. That one also isn't going to cause problems in practice, 
since it only gets executed when a character is both IFS and IFS 
whitespace, but when called from readcmd_handle_line, there's never 
going to be escaped IFS whitespace.

> Thanks.  I've fixed up the buglet causing the failures:

The results are a lot better now, but I did spot a problem:

src/dash -c 'X="x  "; echo $X'

This is supposed to print "x\n", but the IFS breakup of $X generates two 
words, one "x", one " ", meaning "x  \n" gets printed instead. I think 
this is intended to get fixed up in the if (ifsspc) block, but that 
block doesn't get executed when there are no more characters after the 
spaces.

Cheers,
Harald van Dijk

  reply	other threads:[~2016-06-12 10:35 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-02 22:37 dash: read does not ignore trailing spaces Gioele Barabucci
2015-12-02 23:25 ` Jilles Tjoelker
2015-12-03 21:02 ` Harald van Dijk
2015-12-03 21:17   ` Stephane Chazelas
2015-12-03 21:43     ` Martijn Dekker
2015-12-03 23:04       ` Stephane Chazelas
2015-12-03 23:17         ` Stephane Chazelas
2015-12-04  0:00           ` Stephane Chazelas
2015-12-03 22:26     ` Harald van Dijk
2015-12-04 19:51       ` Harald van Dijk
2016-01-29 12:57         ` Martijn Dekker
2016-06-06  8:48         ` [PATCH v2] builtin: Fix handling of trailing IFS white spaces Herbert Xu
2016-06-06 20:43           ` Harald van Dijk
2016-06-07  9:25             ` [PATCH v3] " Herbert Xu
2016-06-12 10:35               ` Harald van Dijk [this message]
2016-06-12 11:06                 ` Herbert Xu
2016-06-12 11:12                   ` Harald van Dijk
2016-06-12 12:17                     ` [PATCH v4] " Herbert Xu
2016-06-19 22:01                       ` Harald van Dijk
2016-06-20  1:28                         ` Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=575D3AE3.1080700@gigawatt.nl \
    --to=harald@gigawatt.nl \
    --cc=dash@vger.kernel.org \
    --cc=gioele@svario.it \
    --cc=herbert@gondor.apana.org.au \
    --cc=stephane.chazelas@gmail.com \
    --subject='Re: [PATCH v3] builtin: Fix handling of trailing IFS white spaces' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).