Thread: wrong shell trap

While messing with the new guc.h stuff I happened run headerscheck and
wanted to abort it right away, and in doing so I realized that its
'trap' line is incorrect: it only removes its temp dir, but it doesn't
exit the program; so after you C-c it, it will spew a ton of complaints
about its temp dir not existing.

AFAICT almost all of our shell scripts contain the same mistake.  I
propose to fix them all as in the attached demo patch, which makes
headerscheck exit properly (no silly noise) when interrupted.

(I confess to not fully understanding why every other trap does
"rm && exit $ret" rather than "rm ; exit", but I guess rm -fr should not
fail anyway thus this should OK.)

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> While messing with the new guc.h stuff I happened run headerscheck and
> wanted to abort it right away, and in doing so I realized that its
> 'trap' line is incorrect: it only removes its temp dir, but it doesn't
> exit the program; so after you C-c it, it will spew a ton of complaints
> about its temp dir not existing.

Ugh.

> AFAICT almost all of our shell scripts contain the same mistake.  I
> propose to fix them all as in the attached demo patch, which makes
> headerscheck exit properly (no silly noise) when interrupted.

Sounds like a good idea.

> (I confess to not fully understanding why every other trap does
> "rm && exit $ret" rather than "rm ; exit", but I guess rm -fr should not
> fail anyway thus this should OK.)

I didn't write these, but I think the idea might be "if rm -rf
fails, report its error status rather than whatever we had before".
However, if that is the idea then it's wrong, because as you've
discovered we won't exit at all unless the trap string does so.
So ISTM that 'ret=$?; rm -rf $tmp; exit $ret' is the correct coding,
or at least less incorrect than either of these alternatives.

            regards, tom lane

On Tue, Sep 13, 2022 at 2:01 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > AFAICT almost all of our shell scripts contain the same mistake.  I
> > propose to fix them all as in the attached demo patch, which makes
> > headerscheck exit properly (no silly noise) when interrupted.
>
> Sounds like a good idea.

Might not be a bad idea to run shellcheck against the scripts, to see
if that highlights anything.

I've found that shellcheck makes working with shell scripts less
terrible, especially when portability is a concern. It can be used to
enforce consistent coding standards that seem pretty well thought out.
It will sometimes produce dubious warnings, of course, but it tends to
mostly have the right idea, most of the time.

-- 
Peter Geoghegan