refactor: Silence "control reaches end of non-void function" (-Wreturn-type) in psbt.cpp #15938

Silence GCC 7 warning "control reaches end of non-void function" (-Wreturn-type) in psbt.cpp.

Context: https://github.com/bitcoin/bitcoin/commit/ef22fe8c1f331b4f13f21a54d12030b92e83fbe7#r33370109

Before this patch:

$ ./configure CC=gcc-7 CXX=g++-7
$ make 2>&1 | grep -A2 "warning: "
leveldb/util/logging.cc:58:40: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
           (v == kMaxUint64/10 && delta > kMaxUint64%10)) {
                                  ~~~~~~^~~~~~~~~~~~~~~
--
leveldb/port/port_posix.cc:60:15: warning: ‘ecx’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   return (ecx & (1 << 20)) != 0;
          ~~~~~^~~~~~~~~~~~
--
psbt.cpp:341:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
$

After this patch:

$ ./configure CC=gcc-7 CXX=g++-7
$ make 2>&1 | grep -A2 "warning: "
leveldb/util/logging.cc:58:40: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
           (v == kMaxUint64/10 && delta > kMaxUint64%10)) {
                                  ~~~~~~^~~~~~~~~~~~~~~
--
leveldb/port/port_posix.cc:60:15: warning: ‘ecx’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   return (ecx & (1 << 20)) != 0;
          ~~~~~^~~~~~~~~~~~
$

Better to return outside the switch, I think, so the full coverage of the switch is still checked via -Wswitch. In that case could assert at the end instead, to show that it's never hit.

@Empact That's a good point! Adjusted accordingly (but without assert). Please re-review :-)

utACK https://github.com/bitcoin/bitcoin/pull/15938/commits/a4a5f0856000c84b5452b4443cfeae57021dccb9

Better to return outside the switch, I think, so the full coverage of the switch is still checked via -Wswitch. In that case could assert at the end instead, to show that it's never hit.

The switch statement covers all possible options, I think it's strange to return a value that can never be returned, if an "unknown" value ends up it can only be due to memory corruption either a software or hardware bug. This would be the place for an assertion.

Generally I agree, but the only callers for this are to output directly to the user in analyzepsbt, so the options are: display "unknown" amongst other data, or fail out.

@laanwj Thanks for reviewing. Now asserting. Please re-review :-)

Could add the same documentation like elsewhere?

git grep 'no default case'

@MarcoFalke Good point! Added. Please re-review :-)