2

In reference to this post on exiting a function in bash

Does a recursive bash function lose its ability to return after descending?

I tried to make simple example function to explain. This function is given a directory and looks for a .foo file with the directories base name. If it does not exist it might just need to be renamed but it needs to find it first. I could rewrite this of course. The find loop is the cause of need for the returns. I am just curious about the recursive nature of bash functions in this case.

given_directory=$"/home/bob"
some_function "$given_directory"

some_function () {
    specific_foo_file=$"$1"/"$(basename "$1")".foo
    if [ -f "$specific_foo_file" ] ; then
        echo "do file stuff"
    else
        find . -name "*.foo"|while file ; do
            if mv "$file" "$specific_foo_file" ; then
                echo "renamed $file to $specific_foo_file"
                some_function $given_directory
                return 0
            else
                echo "could not rename $file"
                return 1
            fi
        done
        echo "can't find any .foo files, oh well"
    fi
}

So far the echo at the end is always called if the rename was successful.

EDIT: For my environment I am using:

-bash-3.2$ uname -a
SunOS XXXXXXXXXX 5.10 Generic_150400-29 sun4v sparc SUNW,SPARC-Enterprise-T5120
Community
  • 1
  • 1
JaredTS486
  • 436
  • 5
  • 16
  • If instead of just `return 0` you only said `return`, you'd have the effect you wanted (since a function exits with its most recent command's exit status by default). – Charles Duffy Nov 24 '15 at 18:46
  • 1
    Also, run this through http://shellcheck.net/ and fix the issues it finds so we don't have to kibitz on the details. :) – Charles Duffy Nov 24 '15 at 18:47
  • It doesn't matter what return code the `find | while` returns with the `echo "can't find ..."` code will *always* run. Nothing prevents it. – Etan Reisner Nov 24 '15 at 18:49
  • Amended my answer. @EtanReisner, good catch, much appreciated. – Charles Duffy Nov 24 '15 at 18:53
  • BTW, you probably want just `specific_foo_file="$1/$(basename "$1").foo"`. `$""` is i18n syntax that looks up the string given in a translation table to see if you have a localized version; it's very unlikely to be relevant here. – Charles Duffy Nov 24 '15 at 18:55
  • @CharlesDuffy Thanks! That website [you linked](http://stackoverflow.com/questions/33901425/does-a-recursive-bash-function-loose-its-ability-to-return-after-descending?noredirect=1#comment55565371_33901425) is an awesome bonus. – JaredTS486 Nov 24 '15 at 18:58
  • BTW, what's the point of recursing here? Strikes me as both nonperformant and dangerous -- you'd eventually blow your stack if you have too many files. – Charles Duffy Nov 24 '15 at 19:16
  • If you just want to rerun the same code over and over until a condition no longer fires, well, that's what a `while` loop is for. – Charles Duffy Nov 24 '15 at 19:23
  • @CharlesDuffy This example was just to give a small idea of what I have already which I was hesitant to post in its entirety. I am trying to write a logical script that is given a # of attempts to try and solve a wide variety of possible issues that could occur when validating the contents of a directory against an .md5 file before giving up. This was one case I got stuck on. Recursion felt natural because after attempting a "fix" I really just needed to run it back through the same process. Here is my [whole function](http://pastie.org/10578749#). – JaredTS486 Nov 24 '15 at 19:39
  • I see you're still doing the `$""` thing. Also, consider using POSIX-compliant function-definition syntax (that is, leave out the `function` keyword, which is non-standards-compliant): `some_function() {`, not `function some_function {`. Also, consider keeping the `local` declarations on different lines from assignment -- otherwise, you get the exit status of `local`, not of any subshell run in the assignment. Also, you're still missing some quotes (shellcheck should catch that). – Charles Duffy Nov 24 '15 at 20:33
  • Given as you have `attempts` limiting the recursion depth, though, the concern about blowing the stack is mooted as long as you maintain a reasonably low upper boundary on that value. – Charles Duffy Nov 24 '15 at 20:34

1 Answers1

3

If you want to pass an erroneous exit status out multiple levels, you need to explicitly pass it through the interim layers. That is:

some_function "$given_directory" || return
  • If some_function succeeds, the function's execution proceeds to the next line.
  • If some_function fails, the function immediately returns with the same exit status used for the failure.

That said, since you have a return 0 as the line immediately following your function call, you could simply change that to a bare return, and have the same effect.

Also, you have an additional serious problem in that your while loop is happening in a subshell, so when it returns, it only impacts that subshell, not the outer shell that invoked it. Instead, restructure your code to put the logic inside the main shell:

while IFS= read -r -d '' file ; do
    if mv -- "$file" "$specific_foo_file" ; then
        echo "renamed $file to $specific_foo_file"
        some_function "$given_directory" || return
    else
        echo "could not rename $file"
        return 1
    fi
done < <(find . -name '*.foo' -print0)

See BashFAQ #24 to understand this issue.

Charles Duffy
  • 280,126
  • 43
  • 390
  • 441
  • @EtanReisner, that's why I discuss both options (of changing the `return 0` to just a `return`, or adding the `|| return`). Certainly, if the `0` is removed from the `return 0`, the `|| return` adds no value. – Charles Duffy Nov 24 '15 at 19:01
  • I'm assuming the OP still has their echo after the `done`, so if we don't return at all, we get a status of 0 from that. Also, that would mean we would continue to process other files after a failure (and eventually reach that echo) rather than short-circuiting. – Charles Duffy Nov 24 '15 at 19:02
  • @CharlesDuffy For my version of find, `-print0` gives me a `find: bad option -print0` any suggestions? – JaredTS486 Nov 24 '15 at 19:14
  • Other than not using `find` relying on shell globbing, not a whole lot that doesn't compromise safety. Which version of bash is this? If it's 4.x, then you can `shopt -s globstar nullglob`, and do a recursive glob: `for file in *.foo **/*.foo; do` – Charles Duffy Nov 24 '15 at 19:16
  • (To be clear what I mean by "compromise safety" -- think about what happens if you have a file named `./tmp/i-am-evil/$'\n'/etc/passwd$'\n'.foo`, with a literal newline in the name; an attacker could trick your script into handling arbitrary files -- if your `find` emits such names literally -- or create files that couldn't be renamed -- if it changes nonprintable characters into printable content). – Charles Duffy Nov 24 '15 at 19:17
  • @JaredTS486, ...that said, if you don't care about safety in the face of evil filenames, take out the `-d ''` on the `read`, and change the `-print0` on the `find` to `-print`. But if you have the choice, I'd suggest switching to shell globs instead. – Charles Duffy Nov 24 '15 at 19:29
  • I completely agree on your safety suggestions and thank you for the suggestion on [globbing.](http://unix.stackexchange.com/questions/162586/proper-way-to-iterate-through-contents-in-a-directory). This is something I have done previous but did not understand the importance. I came up with `for file in "$1"/*.foo; do` for my loop, correct me if it looks wrong but that does seem to work. Back to testing your answer. – JaredTS486 Nov 24 '15 at 20:31
  • Yes, `"$1"/*.foo` is correct as long as you don't need to recurse into subdirectories. – Charles Duffy Nov 24 '15 at 20:35