[PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection

[PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 3706 bytes --]

Replace all uses of cat that do nothing other than read a single file.  In the
case of git-quilt-import, this occurs once per patch.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---

This revised version fixes a bug caught by Stephen Rothwell: the output of wc
-l changes when it has a filename on the command line.  The same bug occurred
in one other place as well.

 git-commit.sh        |    2 +-
 git-filter-branch.sh |    4 ++--
 git-ls-remote.sh     |    2 +-
 git-quiltimport.sh   |    4 ++--
 git-verify-tag.sh    |    3 +--
 5 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index e8b60f7..06b6cd7 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -617,7 +617,7 @@ then
 		tree=$(GIT_INDEX_FILE="$TMP_INDEX" git-write-tree) &&
 		rm -f "$TMP_INDEX"
 	fi &&
-	commit=$(cat "$GIT_DIR"/COMMIT_MSG | git-commit-tree $tree $PARENTS) &&
+	commit=$(git-commit-tree $tree $PARENTS < "$GIT_DIR"/COMMIT_MSG) &&
 	rlogm=$(sed -e 1q "$GIT_DIR"/COMMIT_MSG) &&
 	git-update-ref -m "$GIT_REFLOG_ACTION: $rlogm" HEAD $commit "$current" &&
 	rm -f -- "$GIT_DIR/MERGE_HEAD" "$GIT_DIR/MERGE_MSG" &&
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 0c8a7df..346cf3f 100644
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -333,7 +333,7 @@ for commit in $unchanged; do
 done
 
 git-rev-list --reverse --topo-order $srcbranch --not $unchanged >../revs
-commits=$(cat ../revs | wc -l | tr -d " ")
+commits=$(wc -l ../revs | tr -d -c 0-9)
 
 test $commits -eq 0 && die "Found nothing to rewrite"
 
@@ -386,7 +386,7 @@ while read commit; do
 done <../revs
 
 git-update-ref refs/heads/"$dstbranch" $(head -n 1 ../map/$(tail -n 1 ../revs))
-if [ "$(cat ../map/$(tail -n 1 ../revs) | wc -l)" -gt 1 ]; then
+if [ "$(wc -l < ../map/$(tail -n 1 ../revs))" -gt 1 ]; then
 	echo "WARNING: Your commit filter caused the head commit to expand to several rewritten commits. Only the first such commit was recorded as the current $dstbranch head but you will need to resolve the situation now (probably by manually merging the other commits). These are all the commits:" >&2
 	sed 's/^/	/' ../map/$(tail -n 1 ../revs) >&2
 	ret=1
diff --git a/git-ls-remote.sh b/git-ls-remote.sh
index a6ed99a..f5b2e77 100755
--- a/git-ls-remote.sh
+++ b/git-ls-remote.sh
@@ -82,7 +82,7 @@ rsync://* )
 	(cd $tmpdir && find refs -type f) |
 	while read path
 	do
-		cat "$tmpdir/$path" | tr -d '\012'
+		tr -d '\012' < "$tmpdir/$path"
 		echo "	$path"
 	done &&
 	rm -fr $tmpdir
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index a7a6757..bd540cd 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -70,9 +70,9 @@ tmp_info="$tmp_dir/info"
 commit=$(git-rev-parse HEAD)
 
 mkdir $tmp_dir || exit 2
-for patch_name in $(cat "$QUILT_PATCHES/series" | grep -v '^#'); do
+for patch_name in $(grep -v '^#' "$QUILT_PATCHES/series"); do
 	echo $patch_name
-	(cat $QUILT_PATCHES/$patch_name | git-mailinfo "$tmp_msg" "$tmp_patch" > "$tmp_info") || exit 3
+	git-mailinfo "$tmp_msg" "$tmp_patch" < "$QUILT_PATCHES/$patch_name" > "$tmp_info" || exit 3
 	test -s .dotest/patch || {
 		echo "Patch is empty.  Was it split wrong?"
 		exit 1
diff --git a/git-verify-tag.sh b/git-verify-tag.sh
index 8db7dd0..11ce947 100755
--- a/git-verify-tag.sh
+++ b/git-verify-tag.sh
@@ -38,8 +38,7 @@ trap 'rm -f "$GIT_DIR/.tmp-vtag"' 0
 
 git-cat-file tag "$1" >"$GIT_DIR/.tmp-vtag" || exit 1
 
-cat "$GIT_DIR/.tmp-vtag" |
-sed '/-----BEGIN PGP/Q' |
+sed '/-----BEGIN PGP/Q' "$GIT_DIR/.tmp-vtag" |
 gpg --verify "$GIT_DIR/.tmp-vtag" - || exit 1
 rm -f "$GIT_DIR/.tmp-vtag"
 
-- 
1.5.2.1



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection

[Junio C Hamano]

Josh Triplett <josh@freedesktop.org> writes:

> Replace all uses of cat that do nothing other than read a single file.  In the
> case of git-quilt-import, this occurs once per patch.
>
> Signed-off-by: Josh Triplett <josh@freedesktop.org>
> ---
>
> This revised version fixes a bug caught by Stephen Rothwell: the output of wc
> -l changes when it has a filename on the command line.  The same bug occurred
> in one other place as well.

Hmph...

> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 0c8a7df..346cf3f 100644
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -333,7 +333,7 @@ for commit in $unchanged; do
>  done
>  
>  git-rev-list --reverse --topo-order $srcbranch --not $unchanged >../revs
> -commits=$(cat ../revs | wc -l | tr -d " ")
> +commits=$(wc -l ../revs | tr -d -c 0-9)


... and left unfixed ;-)?

Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]

Junio C Hamano wrote:
> Josh Triplett <josh@freedesktop.org> writes:
>> Replace all uses of cat that do nothing other than read a single file.  In the
>> case of git-quilt-import, this occurs once per patch.
>>
>> Signed-off-by: Josh Triplett <josh@freedesktop.org>
>> ---
>>
>> This revised version fixes a bug caught by Stephen Rothwell: the output of wc
>> -l changes when it has a filename on the command line.  The same bug occurred
>> in one other place as well.
> 
> Hmph...
> 
>> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
>> index 0c8a7df..346cf3f 100644
>> --- a/git-filter-branch.sh
>> +++ b/git-filter-branch.sh
>> @@ -333,7 +333,7 @@ for commit in $unchanged; do
>>  done
>>  
>>  git-rev-list --reverse --topo-order $srcbranch --not $unchanged >../revs
>> -commits=$(cat ../revs | wc -l | tr -d " ")
>> +commits=$(wc -l ../revs | tr -d -c 0-9)
> 
> ... and left unfixed ;-)?

No, just fixed differently. :) Note the change to the tr invocation: delete
everything other than digits.

- Josh Triplett



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection

[Junio C Hamano]

Josh Triplett <josh@freedesktop.org> writes:

> No, just fixed differently. :) Note the change to the tr invocation: delete
> everything other than digits.

Ah, silly me.  Sorry for the noise.

Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection

[Johannes Schindelin]

Hi,

On Wed, 6 Jun 2007, Josh Triplett wrote:

> Junio C Hamano wrote:
> > Josh Triplett <josh@freedesktop.org> writes:
> >> Replace all uses of cat that do nothing other than read a single file.  In the
> >> case of git-quilt-import, this occurs once per patch.
> >>
> >> Signed-off-by: Josh Triplett <josh@freedesktop.org>
> >> ---
> >>
> >> This revised version fixes a bug caught by Stephen Rothwell: the output of wc
> >> -l changes when it has a filename on the command line.  The same bug occurred
> >> in one other place as well.
> > 
> > Hmph...
> > 
> >> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> >> index 0c8a7df..346cf3f 100644
> >> --- a/git-filter-branch.sh
> >> +++ b/git-filter-branch.sh
> >> @@ -333,7 +333,7 @@ for commit in $unchanged; do
> >>  done
> >>  
> >>  git-rev-list --reverse --topo-order $srcbranch --not $unchanged >../revs
> >> -commits=$(cat ../revs | wc -l | tr -d " ")
> >> +commits=$(wc -l ../revs | tr -d -c 0-9)
> > 
> > ... and left unfixed ;-)?
> 
> No, just fixed differently. :) Note the change to the tr invocation: delete
> everything other than digits.

Actually, it feels wrong. For example, if some wc some day decides to 
display the size in kilobyte, even if you say "-l", it would fail badly. 
That is, it would fail to function properly, but would not tell you that 
it failed.

Things like that are known to happen, and that's why "wc -l < file" is a 
better fix than "wc -l file | tr -dc 0-9". In this case, it might not 
matter for a long time, but why not stop being sloppy here and now?

Ciao,
Dscho

Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection

[Martin Langhoff]

On 6/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Things like that are known to happen, and that's why "wc -l < file" is a
> better fix than "wc -l file | tr -dc 0-9". In this case, it might not
> matter for a long time, but why not stop being sloppy here and now?

Not sure if I'd call is sloppy, but I also prefer wc -l < file -- it's
definitely safer.

cheers,


m

Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection

[Johannes Schindelin]

Hi,

On Fri, 8 Jun 2007, Martin Langhoff wrote:

> On 6/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > Things like that are known to happen, and that's why "wc -l < file" is a
> > better fix than "wc -l file | tr -dc 0-9". In this case, it might not
> > matter for a long time, but why not stop being sloppy here and now?
> 
> Not sure if I'd call is sloppy, but I also prefer wc -l < file -- it's
> definitely safer.

Okay, so I sounded harsher than intended.

I should have phrased it like that: I consider "wc -l < file" 
substantially more future-proof, and it is in general a good practice 
IMVHO not to generate something you throw away just after that, when there 
is a way to avoid generating the unwanted part consistently.

Sorry,
Dscho