Re: [OSSTEST PATCH 6/9] db_retry: Suppress an "exiting via last" warning

From: Ian Campbell <ian.campbell@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: xen-devel@lists.xenproject.org
Subject: Re: [OSSTEST PATCH 6/9] db_retry: Suppress an "exiting via last" warning
Date: Fri, 18 Dec 2015 11:14:18 +0000	[thread overview]
Message-ID: <1450437258.4053.198.camel@citrix.com> (raw)
In-Reply-To: <22131.320.250211.170343@mariner.uk.xensource.com>

On Thu, 2015-12-17 at 18:38 +0000, Ian Jackson wrote:

Continuing purely for my own edification...

> FYI, it's (eliding lots) like this:
> 
>   sub db_retry ($$$;$$) {
>       for (;;) {
>           $pre->();
>           eval {
>               $r= &$body;
>               if ($db_retry_stop) {
>                   no warnings qw(exiting);
>                   last if $db_retry_stop eq 'abort';
>                   next;
>               }
>               $dbh->commit();
>           };
>           last if !length $@;
>       }
>       return $r;
>   }
> 
> The warning from perl is because when `last if $db_retry_stop'
> happens, `last' exits the eval in order to exit the outer for (;;)
> loop.

Meaning that last (or next) will exit the eval and immediately end (or
restart) the containing loop, in particular without executing the "last if
!length $@;".

> Both of these are intentional.  Perl thinks they might not be.

... and that's because it is a common enough mistake to think that next or
last exits only the scope of the eval and therefore in this case to expect
it to run "last if !length $@;", while it actually wont.

I would have expected that if you want to successfully exit (i.e. not die)
the eval early and continue on with the remainder of the loop you should
use return, and http://perldoc.perl.org/functions/eval.html seems to back
that up ("the value returned is the value of the last expression evaluated
inside the mini-program; a return statement may be also used,"), but
earlier you said that return would generate the same warning, so I remain a
bit confused.

According to my reading of eval.html "return if $db_retry_stop eq 'abort'"
would exit the eval with $@="" ("If there was no error, $@ is set to the
empty string", it mentions last and goto bypassing this, but not return).
Then since length "" is false that outer last would be run insteadand we
are done with the loop as we wanted. AFAICT that would avoid the warning
for the inner last, but is not usable for the inner next, and in any case
using last inside the eval is actually more obvious in this instance, once
one thinks one understands an eval (which in my case is still debatable I
think)

Ian.

> Thanks for all the acks.  I'll maybe push this at some point, or
> combine it with something else.  It's not urgent.
> 
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel