Libguestfs - Aug 2014 - [PATCH] lua: always return luaL_error in actions

Even if luaL_error is a "no return" function for the Lua runtime,
adopt
also in action functions the "return" idiom recommeded for it.

This also helps code analyzers in not thinking that "g" might still be
null after the null check followed by luaL_error.
---
 generator/lua.ml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/generator/lua.ml b/generator/lua.ml
index 9bd4006..5d5619c 100644
--- a/generator/lua.ml
+++ b/generator/lua.ml
@@ -482,8 +482,8 @@ guestfs_lua_delete_event_callback (lua_State *L)
       pr "\n";
 
       pr "  if (g == NULL)\n";
-      pr "    luaL_error (L, \"Guestfs.%%s: handle is
closed\",\n";
-      pr "                \"%s\");\n" name;
+      pr "    return luaL_error (L, \"Guestfs.%%s: handle is
closed\",\n";
+      pr "                       \"%s\");\n" name;
       pr "\n";
 
       iteri (
-- 
1.9.3

On Thu, Aug 07, 2014 at 07:48:08PM +0200, Pino Toscano
wrote:
> Even if luaL_error is a "no return" function for the Lua runtime,
adopt
> also in action functions the "return" idiom recommeded for it.
> 
> This also helps code analyzers in not thinking that "g" might
still be
> null after the null check followed by luaL_error.
> ---
>  generator/lua.ml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/generator/lua.ml b/generator/lua.ml
> index 9bd4006..5d5619c 100644
> --- a/generator/lua.ml
> +++ b/generator/lua.ml
> @@ -482,8 +482,8 @@ guestfs_lua_delete_event_callback (lua_State *L)
>        pr "\n";
>  
>        pr "  if (g == NULL)\n";
> -      pr "    luaL_error (L, \"Guestfs.%%s: handle is
closed\",\n";
> -      pr "                \"%s\");\n" name;
> +      pr "    return luaL_error (L, \"Guestfs.%%s: handle is
closed\",\n";
> +      pr "                       \"%s\");\n" name;
>        pr "\n";
Yeah, I opened a ticket to get the Coverity model changed several
years ago, but nothing happened.

So this looks the best we can do => ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Thursday 07 August 2014 20:57:50 Richard W.M. Jones
wrote:
> On Thu, Aug 07, 2014 at 07:48:08PM +0200, Pino Toscano wrote:
> > Even if luaL_error is a "no return" function for the Lua
runtime,
> > adopt also in action functions the "return" idiom recommeded
for
> > it.
> > 
> > This also helps code analyzers in not thinking that "g"
might still
> > be null after the null check followed by luaL_error.
> > ---
> > 
> >  generator/lua.ml | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/generator/lua.ml b/generator/lua.ml
> > index 9bd4006..5d5619c 100644
> > --- a/generator/lua.ml
> > +++ b/generator/lua.ml
> > @@ -482,8 +482,8 @@ guestfs_lua_delete_event_callback (lua_State *L)
> > 
> >        pr "\n";
> >        
> >        pr "  if (g == NULL)\n";
> > 
> > -      pr "    luaL_error (L, \"Guestfs.%%s: handle is
closed\",\n";
> > -      pr "                \"%s\");\n" name;
> > +      pr "    return luaL_error (L, \"Guestfs.%%s: handle
is
> > closed\",\n"; +      pr "                      
\"%s\");\n" name;
> > 
> >        pr "\n";
> 
> Yeah, I opened a ticket to get the Coverity model changed several
> years ago, but nothing happened.
On the other hand, neither luaL_error nor lua_error (called internally 
by the former) have any attribute on them, so the static analyser cannot 
know they will jump back to some point within the interpreter.

Unless, of course, the analyser has some override to know that.

-- 
Pino Toscano