threads / patch / 46583

patchstash: prevent warning about null bytes in input

Subject: [PATCH] stash: prevent warning about null bytes in input

## tl;dr

5 messages between Aug 14, 2017 and Aug 14, 2017. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Kevin Daudt· Aug 14, 2017, 05:08 UTC · lore

The no_changes function calls the untracked_files function through command substitution. untracked_files will return null bytes because it runs ls-files with the '-z' option.

Bash since version 4.4 warns about these null bytes. As they are not required for the test that is being done, remove null bytes from the input.

This warning is triggered when running git stash save -u resulting in two warnings:

    git-stash: line 43: warning: command substitution: ignored null byte
    in input

Signed-off-by: Kevin Daudt <me@ikke.info>
---
 git-stash.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to git-stash.sh +1 −1

diff --git a/git-stash.sh b/git-stash.sh
index 9b6c2da7b..0dcca3cd6 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -39,7 +39,7 @@ fi
 no_changes () {
 	git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
 	git diff-files --quiet --ignore-submodules -- "$@" &&
-	(test -z "$untracked" || test -z "$(untracked_files)")
+	(test -z "$untracked" || test -z "$(untracked_files | tr -d '\0')")
 }
 
 untracked_files () {

-- 
2.14.0.rc1.33.g384a8b271c

Junio C Hamano· Aug 14, 2017, 19:51 UTC · re: Kevin Daudt · lore

Re: [PATCH] stash: prevent warning about null bytes in input

Kevin Daudt <me@ikke.info> writes:

Show 7 quoted lines

> The no_changes function calls the untracked_files function through
> command substitution. untracked_files will return null bytes because it
> runs ls-files with the '-z' option.
>
> Bash since version 4.4 warns about these null bytes. As they are not
> required for the test that is being done, remove null bytes from the
> input.

That's an interesting one ;-)

I wonder if you considered giving an option to untracked_files helper function, though. After all, it has only two callers, and it feels a bit suboptimal to ask the command to do a special thing (i.e. "-z") only to clean it up with a pipe.

IOW, something along the lines of (totally untested)...

 git-stash.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Show changes to git-stash.sh +9 −2

diff --git a/git-stash.sh b/git-stash.sh
index 9b6c2da7b4..5f09a47f0a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -43,9 +43,16 @@ no_changes () {
 }
 
 untracked_files () {
+	if test "$1" = "-z"
+	then
+		shift
+		z=-z
+	else
+		z=
+	fi
 	excl_opt=--exclude-standard
 	test "$untracked" = "all" && excl_opt=
-	git ls-files -o -z $excl_opt -- "$@"
+	git ls-files -o $z $excl_opt -- "$@"
 }
 
 clear_stash () {
@@ -114,7 +121,7 @@ create_stash () {
 		# Untracked files are stored by themselves in a parentless commit, for
 		# ease of unpacking later.
 		u_commit=$(
-			untracked_files "$@" | (
+			untracked_files -z "$@" | (
 				GIT_INDEX_FILE="$TMPindex" &&
 				export GIT_INDEX_FILE &&
 				rm -f "$TMPindex" &&

Kevin Daudt· Aug 14, 2017, 20:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] stash: prevent warning about null bytes in input

On Mon, Aug 14, 2017 at 12:51:26PM -0700, Junio C Hamano wrote:

Show 16 quoted lines

> Kevin Daudt <me@ikke.info> writes:
> 
> > The no_changes function calls the untracked_files function through
> > command substitution. untracked_files will return null bytes because it
> > runs ls-files with the '-z' option.
> >
> > Bash since version 4.4 warns about these null bytes. As they are not
> > required for the test that is being done, remove null bytes from the
> > input.
> 
> That's an interesting one ;-)
> 
> I wonder if you considered giving an option to untracked_files
> helper function, though.  After all, it has only two callers,
> and it feels a bit suboptimal to ask the command to do a special
> thing (i.e. "-z") only to clean it up with a pipe.

As a matter of fact, I did not consider that option. I do agree that's a much better approach.

> 
> IOW, something along the lines of (totally untested)...
>

How should I proceed with this? Resubmit it after testing with the appropriate attribution?

Show 36 quoted lines

>  git-stash.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index 9b6c2da7b4..5f09a47f0a 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -43,9 +43,16 @@ no_changes () {
>  }
>  
>  untracked_files () {
> +	if test "$1" = "-z"
> +	then
> +		shift
> +		z=-z
> +	else
> +		z=
> +	fi
>  	excl_opt=--exclude-standard
>  	test "$untracked" = "all" && excl_opt=
> -	git ls-files -o -z $excl_opt -- "$@"
> +	git ls-files -o $z $excl_opt -- "$@"
>  }
>  
>  clear_stash () {
> @@ -114,7 +121,7 @@ create_stash () {
>  		# Untracked files are stored by themselves in a parentless commit, for
>  		# ease of unpacking later.
>  		u_commit=$(
> -			untracked_files "$@" | (
> +			untracked_files -z "$@" | (
>  				GIT_INDEX_FILE="$TMPindex" &&
>  				export GIT_INDEX_FILE &&
>  				rm -f "$TMPindex" &&
> 
>

Junio C Hamano· Aug 14, 2017, 20:37 UTC · re: Kevin Daudt · lore

Re: [PATCH] stash: prevent warning about null bytes in input

Kevin Daudt <me@ikke.info> writes:

Show 27 quoted lines

> On Mon, Aug 14, 2017 at 12:51:26PM -0700, Junio C Hamano wrote:
>> Kevin Daudt <me@ikke.info> writes:
>> 
>> > The no_changes function calls the untracked_files function through
>> > command substitution. untracked_files will return null bytes because it
>> > runs ls-files with the '-z' option.
>> >
>> > Bash since version 4.4 warns about these null bytes. As they are not
>> > required for the test that is being done, remove null bytes from the
>> > input.
>> 
>> That's an interesting one ;-)
>> 
>> I wonder if you considered giving an option to untracked_files
>> helper function, though.  After all, it has only two callers,
>> and it feels a bit suboptimal to ask the command to do a special
>> thing (i.e. "-z") only to clean it up with a pipe.
>
> As a matter of fact, I did not consider that option. I do agree that's a
> much better approach.
>
>> 
>> IOW, something along the lines of (totally untested)...
>> 
>
> How should I proceed with this? Resubmit it after testing with the
> appropriate attribution?

Sure.

An appropriate attribution would be a "Helped-by: me" at most, but I do not think for something this small it may not even bee needed.

Show 37 quoted lines

>
>>  git-stash.sh | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/git-stash.sh b/git-stash.sh
>> index 9b6c2da7b4..5f09a47f0a 100755
>> --- a/git-stash.sh
>> +++ b/git-stash.sh
>> @@ -43,9 +43,16 @@ no_changes () {
>>  }
>>  
>>  untracked_files () {
>> +	if test "$1" = "-z"
>> +	then
>> +		shift
>> +		z=-z
>> +	else
>> +		z=
>> +	fi
>>  	excl_opt=--exclude-standard
>>  	test "$untracked" = "all" && excl_opt=
>> -	git ls-files -o -z $excl_opt -- "$@"
>> +	git ls-files -o $z $excl_opt -- "$@"
>>  }
>>  
>>  clear_stash () {
>> @@ -114,7 +121,7 @@ create_stash () {
>>  		# Untracked files are stored by themselves in a parentless commit, for
>>  		# ease of unpacking later.
>>  		u_commit=$(
>> -			untracked_files "$@" | (
>> +			untracked_files -z "$@" | (
>>  				GIT_INDEX_FILE="$TMPindex" &&
>>  				export GIT_INDEX_FILE &&
>>  				rm -f "$TMPindex" &&
>> 
>>

Kevin Daudt· Aug 14, 2017, 21:43 UTC · re: Kevin Daudt · lore

[PATCH v2] stash: prevent warning about null bytes in input

The `no_changes` function calls the `untracked_files` function through command substitution. `untracked_files` will return null bytes because it runs ls-files with the '-z' option.

Bash since version 4.4 warns about these null bytes. As they are not required for the test that is being done, make sure `untracked_files` does not output null bytes when not required.

This is achieved by adding a parameter to the `untracked_files` function to specify wither `-z` should be passed to ls-files or not.

This warning is triggered when running git stash save -u resulting in two warnings:

    git-stash: line 43: warning: command substitution: ignored null byte
    in input

Signed-off-by: Kevin Daudt <me@ikke.info>
---
 git-stash.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Show changes to git-stash.sh +9 −2

diff --git a/git-stash.sh b/git-stash.sh
index 9b6c2da7b..5f09a47f0 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -43,9 +43,16 @@ no_changes () {
 }
 
 untracked_files () {
+	if test "$1" = "-z"
+	then
+		shift
+		z=-z
+	else
+		z=
+	fi
 	excl_opt=--exclude-standard
 	test "$untracked" = "all" && excl_opt=
-	git ls-files -o -z $excl_opt -- "$@"
+	git ls-files -o $z $excl_opt -- "$@"
 }
 
 clear_stash () {
@@ -114,7 +121,7 @@ create_stash () {
 		# Untracked files are stored by themselves in a parentless commit, for
 		# ease of unpacking later.
 		u_commit=$(
-			untracked_files "$@" | (
+			untracked_files -z "$@" | (
 				GIT_INDEX_FILE="$TMPindex" &&
 				export GIT_INDEX_FILE &&
 				rm -f "$TMPindex" &&

-- 
2.14.1.145.gb3622a4ee9