PATCH: Fix handling of NUL bytes in zexpandtabs multibyte version

PATCH: Fix handling of NUL bytes in zexpandtabs multibyte version

[Mikael Magnusson]

Problem found by dg1727 on #zsh.

---

I checked other uses of zexpandtabs in the same file only and they all
seem to handle the 0 return correctly, and some of them even comment on
it, so fairly confident this is the right fix.

---
 Src/utils.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Src/utils.c b/Src/utils.c
index 8fcea5362a..f8cbe65ba5 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -5936,8 +5936,11 @@ zexpandtabs(const char *s, int len, int width, int startpos, FILE *fout,
 		memset(&mbs, 0, sizeof(mbs));
 		s++;
 		len--;
-	    } else if (ret == MB_INCOMPLETE) {
+	    } else if (ret == MB_INCOMPLETE ||
 		/* incomplete at end --- assume likewise, best we've got */
+	               ret == 0) {
+		/* NUL character returns 0, which would loop infinitely, so advance
+		 * one byte in this case too */
 		s++;
 		len--;
 	    } else {
-- 
2.15.1

Re: PATCH: Fix handling of NUL bytes in zexpandtabs multibyte version

[Mikael Magnusson]

On 3/12/21, Mikael Magnusson <mikachu@gmail.com> wrote:
> Problem found by dg1727 on #zsh.

Forgot the actual problem part,

print -x2 '\0'

is the reduced code that causes it.

Re: PATCH: Fix handling of NUL bytes in zexpandtabs multibyte version

[Daniel Shahaf]

Mikael Magnusson wrote on Fri, 12 Mar 2021 03:17 +00:00:
> On 3/12/21, Mikael Magnusson <mikachu@gmail.com> wrote:
> > Problem found by dg1727 on #zsh.
> 
> Forgot the actual problem part,
> 
> print -x2 '\0'
> 
> is the reduced code that causes it.

Regression test?

Re: PATCH: Fix handling of NUL bytes in zexpandtabs multibyte version

[Lawrence Velázquez]

On Fri, Mar 12, 2021, at 8:53 AM, Daniel Shahaf wrote:
> Mikael Magnusson wrote on Fri, 12 Mar 2021 03:17 +00:00:
> > On 3/12/21, Mikael Magnusson <mikachu@gmail.com> wrote:
> > > Problem found by dg1727 on #zsh.
> > 
> > Forgot the actual problem part,
> > 
> > print -x2 '\0'
> > 
> > is the reduced code that causes it.
> 
> Regression test?

ping

vq

PATCH v2: 48168: Fix handling of NUL bytes in zexpandtabs multibyte version

[Mikael Magnusson]

---

 Src/utils.c        | 5 ++++-
 Test/B03print.ztst | 9 +++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/Src/utils.c b/Src/utils.c
index 8fcea5362a..f8cbe65ba5 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -5936,8 +5936,11 @@ zexpandtabs(const char *s, int len, int width, int startpos, FILE *fout,
 		memset(&mbs, 0, sizeof(mbs));
 		s++;
 		len--;
-	    } else if (ret == MB_INCOMPLETE) {
+	    } else if (ret == MB_INCOMPLETE ||
 		/* incomplete at end --- assume likewise, best we've got */
+	               ret == 0) {
+		/* NUL character returns 0, which would loop infinitely, so advance
+		 * one byte in this case too */
 		s++;
 		len--;
 	    } else {
diff --git a/Test/B03print.ztst b/Test/B03print.ztst
index 5634239346..d8618de7d4 100644
--- a/Test/B03print.ztst
+++ b/Test/B03print.ztst
@@ -304,15 +304,16 @@
  foo=$'one\ttwo\tthree\tfour\n'
  foo+=$'\tone\ttwo\tthree\tfour\n'
  foo+=$'\t\tone\t\ttwo\t\tthree\t\tfour'
- print -x4 $foo
- print -X4 $foo
+ foo+='\0' # regression test for multibyte tab expand
+ print -x4 $foo | sed 's/\x00/nul/' # avoid raw nul byte in expected output below
+ print -X4 $foo | sed 's/\x00/nul/'
 0:Tab expansion by print
 >one	two	three	four
 >    one	two	three	four
->        one		two		three		four
+>        one		two		three		fournul
 >one two three   four
 >    one two three   four
->        one     two     three       four
+>        one     two     three       fournul
 
  unset foo
  print -v foo once more
-- 
2.15.1

Re: PATCH v2: 48168: Fix handling of NUL bytes in zexpandtabs multibyte version

[Lawrence Velázquez]

On Sat, Mar 20, 2021, at 11:15 AM, Mikael Magnusson wrote:
> ---
> 
>  Src/utils.c        | 5 ++++-
>  Test/B03print.ztst | 9 +++++----
>  2 files changed, 9 insertions(+), 5 deletions(-)

ping for review

vq

Re: PATCH v2: 48168: Fix handling of NUL bytes in zexpandtabs multibyte version

[Jun T]

The test in B03 fails on macOS and FreeBSD (and maybe on other BSDs)
because 'sed' can't handle the null byte.

It seems 'tr' is more portable.


diff --git a/Test/B03print.ztst b/Test/B03print.ztst
index 563423934..4d2cf9764 100644
--- a/Test/B03print.ztst
+++ b/Test/B03print.ztst
@@ -304,15 +304,16 @@
  foo=$'one\ttwo\tthree\tfour\n'
  foo+=$'\tone\ttwo\tthree\tfour\n'
  foo+=$'\t\tone\t\ttwo\t\tthree\t\tfour'
- print -x4 $foo
- print -X4 $foo
+ foo+='\0' # regression test for multibyte tab expand
+ print -x4 $foo | tr '\0' Z # avoid raw nul byte in expected output below
+ print -X4 $foo | tr '\0' Z
 0:Tab expansion by print
 >one	two	three	four
 >    one	two	three	four
->        one		two		three		four
+>        one		two		three		fourZ
 >one two three   four
 >    one two three   four
->        one     two     three       four
+>        one     two     three       fourZ
 
  unset foo
  print -v foo once more

Re: PATCH v2: 48168: Fix handling of NUL bytes in zexpandtabs multibyte version

[Mikael Magnusson]

On 4/1/21, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
> The test in B03 fails on macOS and FreeBSD (and maybe on other BSDs)
> because 'sed' can't handle the null byte.
>
> It seems 'tr' is more portable.
>
>
> diff --git a/Test/B03print.ztst b/Test/B03print.ztst
> index 563423934..4d2cf9764 100644
> --- a/Test/B03print.ztst
> +++ b/Test/B03print.ztst
> @@ -304,15 +304,16 @@
>   foo=$'one\ttwo\tthree\tfour\n'
>   foo+=$'\tone\ttwo\tthree\tfour\n'
>   foo+=$'\t\tone\t\ttwo\t\tthree\t\tfour'
> - print -x4 $foo
> - print -X4 $foo
> + foo+='\0' # regression test for multibyte tab expand
> + print -x4 $foo | tr '\0' Z # avoid raw nul byte in expected output below
> + print -X4 $foo | tr '\0' Z
>  0:Tab expansion by print
>  >one	two	three	four
>  >    one	two	three	four
> ->        one		two		three		four
> +>        one		two		three		fourZ
>  >one two three   four
>  >    one two three   four
> ->        one     two     three       four
> +>        one     two     three       fourZ
>
>   unset foo
>   print -v foo once more

Thanks for checking, I'll use this version instead.

-- 
Mikael Magnusson