syntax: keep "exit guards" in place

[powerman]

https://docs.fedoraproject.org/en-US/Fedora_Security_Team/1/html/Defensive_Coding/sect-Defensive_Coding-Shell-Edit_Guard.html recommends to have main "$@" ; exit $? as single line… Maybe it's worth to support this case and avoid auto-rewriting it as two lines?

[powerman]

BTW, there are other similar workarounds, but they also require two things on same line, e.g.:

{
  # code here
}; exit $?;

[mvdan]

The formatter doesn't try to interpret or understand your code in any way. So we can't treat main "$@"; exit 1 any different than any other pair of commands like foo; bar.

However, you might be interested in #261, which is a more generic solution we could actually implement. If you configured the formatter to allow two consecutive commands on a single line, then I think your single-liner would work.

[powerman]

Yeah, I think solution for #261 will works here too.

[mvdan]

Thanks! Closing in favor of that one then.

[theclapp]

This is an interesting feature of bash, and an interesting possible-bug because of it.

Wouldn't putting exit at the end of your main() work just fine, though?

function main {
  # ... code ...
  exit $?
}

main

Bash has to read the entirety of the main function to execute it, so there's no way editing the script while it's running could remove it or do something after main exits.

[powerman]

@theclapp I doubt this will work. AFAIK bash updates file seek position after executing each line - to support unpacking shell-archives located at same file. So, I suppose while executing main() it'll do seek() after each line, and thus will be affected by the issue. Two things should be at same line to avoid this - it won't seek() at the middle of the line (I suppose).

[theclapp]

So, I suppose while executing main() it'll do seek() after each line

No, it reads all of main at once, stores it, and then executes it when called. If this wasn't true, then the trick of writing main "$@" ; exit $? wouldn't work anyway, because what if you just updated the definition of main while it was running?

In any case, fwiw, I tried it (GNU bash, version 5.0.16(1)-release (x86_64-apple-darwin18.7.0)), and it does work.

For anyone following along at home: If you want to try to reproduce this, be sure your editor writes files in place. E.g. in Vim set nobackup nowritebackup. Otherwise, when you save, it writes to a new file, and then unlinks the old file and renames the new one on top of it. But bash keeps the file open and reads the old (now deleted) one, so you haven't actually updated the file that bash is actually reading. You can check that it's the exact same file by checking the inode number via ls -li filename.

[mvdan]

I think it was wrong to close this issue as a duplicate of #261; re-reading both again, #261 was mostly about preference or semantics, while this is purely about defensive coding.

Personally, I think @theclapp's solution is the sanest. It works with the current formatting rules - it doesn't require placing two commands in a single line.

If we tried to go the opposite route, and somehow detect some-command; exit some-args, that could get complex really fast. exit is not special syntax, it's just a command like any other. We would fail to detect foo; "exit" $? and cmd=exit; foo; $cmd $?. Similarly, we likely can't predict what the previous command will look like, so I don't know what we could possibly do with echo foo; exit and such.

@powerman thoughts?

[mvdan]

Also, I'm open to ideas on how we could detect the "safe pattern" of having two commands in a single line, without allowing any foo; bar line to stay as a single line.

[powerman]

I don't have personal preferences, any working solution is good for me.
But:

• If we're going against well-known and documented everywhere main; exit trick and replacing it with main(){ …; exit }, then we should make sure it'll actually works in every actual shell (including outdated but still popular versions - i.e. don't test just latest bash on MacOS).
• It may worth to add auto-detection of some popular ways to write main; exit and instead of auto-rewriting such scripts print error with url to wiki page which explains how to manually rewrite this part in supported way before running this tool again.

[mvdan]

What you say makes sense, but I still don't know how to reliably detect this without turning off line splitting entirely.

[powerman]

I belive we don't have to implement this reliably - best effort (e.g. few regexp) should be good enough and much better than current behaviour anyway.

[mvdan]

So, quite literally, just matching any-command; exit any-arguments and letting that be a single line?

[powerman]

Maybe even ;\s*exit (see above in comments example with code block).

[mvdan]

I would really appreciate if people gave real examples of these multi-command lines used for safety. So far, I've only been shown some-command; exit $?, so that would lead me to think that we should only worry about that pattern.

Are there others?

[skyzyx]

Here's my snippet:

if [ ! -f files/SHA256SUMS ]; then echo "files/SHA256SUMS file missing." 1>&2; exit 1; fi

shfmt formats this to:

if [ ! -f files/SHA256SUMS ]; then
    echo "files/SHA256SUMS file missing." 1>&2
    exit 1
fi

My co-worker hates this formatting for certain validation rules. I don't give a shit. I'm looking to see if there is a way to annotate a file/line to be left alone. This would allow us to use the good parts of shfmt and shellcheck without causing a fight on the team over something I don't care enough about to fight over.

Trying to find a solution, I got bounced from #564 → #261 → #564, and I still can't tell if there's a solution to my issue or not.

Is there a straight answer I've missed?

[mvdan]

There is no support for annotations or to turn off the formatting for parts of a script, no. There are no plans for that either.

This issue will likely get fixed, because exit guards do serve a purpose. Then, your original line would likely format as:

if [ ! -f files/SHA256SUMS ]; then
    echo "files/SHA256SUMS file missing." 1>&2; exit 1
fi

[kaey]

This specific code can be rewritten as

die() {
    echo "$*" >&2
    exit 1
}

try() {
    "$@" || die "failed: $*"
}

try test -f files/SHA256SUMS
or
test -f files/SHA256SUMS || die "files/SHA256SUMS is missing"

This issue is about guarding against in-place edits while script is running.

[skyzyx]

There is no support for annotations or to turn off the formatting for parts of a script, no. There are no plans for that either.

This issue will likely get fixed, because exit guards do serve a purpose. Then, your original line would likely format as:

if [ ! -f files/SHA256SUMS ]; then
    echo "files/SHA256SUMS file missing." 1>&2; exit 1
fi

That's unfortunate. Guess I'll need to roll up my sleeves and get in a fight.

Thanks.