# [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection 7 messages from 2007-06-06 to 2007-06-08. Participants: Josh Triplett, Junio C Hamano, Johannes Schindelin, Martin Langhoff. Thread: https://gitlist.dev/t/8462 ## Josh Triplett, 2007-06-06 04:36 Subject: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection Message-ID: <466639D0.1040306@freedesktop.org> URL: https://gitlist.dev/e/466639D0.1040306%40freedesktop.org ``` 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 --- 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 ``` ## Junio C Hamano, 2007-06-07 02:09 Subject: Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection Message-ID: <7vsl94sego.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vsl94sego.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <466639D0.1040306@freedesktop.org> ``` Josh Triplett 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 > --- > > 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 ;-)? ``` ## Josh Triplett, 2007-06-07 02:23 Subject: Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection Message-ID: <46676C35.60406@freedesktop.org> URL: https://gitlist.dev/e/46676C35.60406%40freedesktop.org In-Reply-To: <7vsl94sego.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Josh Triplett 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 >> --- >> >> 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 ``` ## Junio C Hamano, 2007-06-07 02:52 Subject: Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection Message-ID: <7vtztkqxvy.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vtztkqxvy.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <46676C35.60406@freedesktop.org> ``` Josh Triplett writes: > No, just fixed differently. :) Note the change to the tr invocation: delete > everything other than digits. Ah, silly me. Sorry for the noise. ``` ## Johannes Schindelin, 2007-06-07 04:06 Subject: Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0706070506230.4046%40racer.site In-Reply-To: <46676C35.60406@freedesktop.org> ``` Hi, On Wed, 6 Jun 2007, Josh Triplett wrote: > Junio C Hamano wrote: > > Josh Triplett 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 > >> --- > >> > >> 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 ``` ## Martin Langhoff, 2007-06-08 10:42 Subject: Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection Message-ID: <46a038f90706080342j3a5bb7abv58c78d7b532e29de@mail.gmail.com> URL: https://gitlist.dev/e/46a038f90706080342j3a5bb7abv58c78d7b532e29de%40mail.gmail.com In-Reply-To: ``` On 6/7/07, Johannes Schindelin 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 ``` ## Johannes Schindelin, 2007-06-08 12:52 Subject: Re: [PATCH v2] Remove useless uses of cat, and replace with filename arguments or redirection Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0706081352260.4059%40racer.site In-Reply-To: <46a038f90706080342j3a5bb7abv58c78d7b532e29de@mail.gmail.com> ``` Hi, On Fri, 8 Jun 2007, Martin Langhoff wrote: > On 6/7/07, Johannes Schindelin 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 ```