[vlc-commits] [Git][videolan/vlc][master] 10 commits: lua: extension: refactor error as early return

Steve Lhomme (@robUx4) gitlab at videolan.org
Tue Feb 14 09:54:55 UTC 2023



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
7c645c8c by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: refactor error as early return

This commit doesn't change indentation just yet, to make the change more
obvious.

- - - - -
b78c0e12 by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: refactor error as early return

This commit doesn't change indentation just yet, to make the change more
obvious.

- - - - -
60e981eb by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: reindent after previous changes

Previous changes were transforming the error path as early return,
leaving a potential indentation level for each commit that could be
removed. This commit is removing them.

No functional changes.

- - - - -
d9e7889a by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: don't cast calloc

calloc returns a `void*` which is automatically casted into any pointer
type in C code.

- - - - -
2af07864 by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: refactor error as early return

This commit doesn't change indentation just yet, to make the change more
obvious.

- - - - -
822f5b41 by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: reindent after previous changes

Previous changes were transforming the error path as early return,
leaving a potential indentation level to be removed. This commit is
removing it.

No functional changes.

- - - - -
cca0ff9c by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: refactor error path to goto

Split the normal path and error path and move the release behind the
error label to remove the handling of the `b_ok` state in the normal
path.

Since returning an error only makes the processing forward to the next
extension to be run, and doesn't fail completely, the label is named
"discard" to signal it will only discard the current extension.

- - - - -
3d5f263c by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: refactor early return

This commit doesn't change indentation just yet, to make the change more
obvious.

- - - - -
8ca59d74 by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: refactor early return

This commit doesn't change indentation just yet, to make the change more
obvious.

- - - - -
fe1c31bc by Alexandre Janniaux at 2023-02-14T09:36:23+00:00
lua: extension: reindent after previous changes

Previous changes were transforming the error path as early return,
leaving a potential indentation level for each commit that could be
removed. This commit is removing them.

No functional changes.

- - - - -


1 changed file:

- modules/lua/extension.c


Changes:

=====================================
modules/lua/extension.c
=====================================
@@ -273,7 +273,6 @@ int ScanLuaCallback( vlc_object_t *p_this, const char *psz_filename,
 {
     VLC_UNUSED(dummy);
     extensions_manager_t *p_mgr = ( extensions_manager_t* ) p_this;
-    bool b_ok = false;
 
     msg_Dbg( p_mgr, "Scanning Lua script %s", psz_filename );
 
@@ -339,7 +338,7 @@ int ScanLuaCallback( vlc_object_t *p_this, const char *psz_filename,
         msg_Warn( p_mgr, "Error loading script %s: %s", psz_script,
                   lua_tostring( L, lua_gettop( L ) ) );
         lua_pop( L, 1 );
-        goto exit;
+        goto discard;
     }
 
     /* Scan script for capabilities */
@@ -349,7 +348,7 @@ int ScanLuaCallback( vlc_object_t *p_this, const char *psz_filename,
     {
         msg_Warn( p_mgr, "Error while running script %s, "
                   "function descriptor() not found", psz_script );
-        goto exit;
+        goto discard;
     }
 
     if( lua_pcall( L, 0, 1, 0 ) )
@@ -357,125 +356,117 @@ int ScanLuaCallback( vlc_object_t *p_this, const char *psz_filename,
         msg_Warn( p_mgr, "Error while running script %s, "
                   "function descriptor(): %s", psz_script,
                   lua_tostring( L, lua_gettop( L ) ) );
-        goto exit;
+        goto discard;
     }
 
-    if( lua_gettop( L ) )
+    if (lua_gettop(L) == 0)
     {
-        if( lua_istable( L, -1 ) )
+        msg_Err(p_mgr, "Script %s went completely foobar", psz_script);
+        goto discard;
+    }
+
+    if (!lua_istable(L, -1))
+    {
+        msg_Warn(p_mgr, "In script %s, function descriptor() "
+                 "did not return a table!", psz_script);
+        goto discard;
+    }
+
+    /* Get caps */
+    lua_getfield(L, -1, "capabilities");
+    if (lua_istable(L, -1))
+    {
+        lua_pushnil(L);
+        while (lua_next(L, -2) != 0)
         {
-            /* Get caps */
-            lua_getfield( L, -1, "capabilities" );
-            if( lua_istable( L, -1 ) )
+            /* Key is at index -2 and value at index -1. Discard key */
+            const char *psz_cap = luaL_checkstring(L, -1);
+            bool found = false;
+            /* Find this capability's flag */
+            for (size_t i = 0; i < ARRAY_SIZE(caps); i++)
             {
-                lua_pushnil( L );
-                while( lua_next( L, -2 ) != 0 )
+                if (!strcmp(caps[i], psz_cap))
                 {
-                    /* Key is at index -2 and value at index -1. Discard key */
-                    const char *psz_cap = luaL_checkstring( L, -1 );
-                    bool found = false;
-                    /* Find this capability's flag */
-                    for( size_t i = 0; i < ARRAY_SIZE(caps); i++ )
-                    {
-                        if( !strcmp( caps[i], psz_cap ) )
-                        {
-                            /* Flag it! */
-                            sys->i_capabilities |= 1 << i;
-                            found = true;
-                            break;
-                        }
-                    }
-                    if( !found )
-                    {
-                        msg_Warn( p_mgr, "Extension capability '%s' unknown in"
-                                  " script %s", psz_cap, psz_script );
-                    }
-                    /* Removes 'value'; keeps 'key' for next iteration */
-                    lua_pop( L, 1 );
+                    /* Flag it! */
+                    sys->i_capabilities |= 1 << i;
+                    found = true;
+                    break;
                 }
             }
-            else
-            {
-                msg_Warn( p_mgr, "In script %s, function descriptor() "
-                              "did not return a table of capabilities.",
-                              psz_script );
-            }
-            lua_pop( L, 1 );
-
-            /* Get title */
-            lua_getfield( L, -1, "title" );
-            if( lua_isstring( L, -1 ) )
+            if (!found)
             {
-                p_ext->psz_title = strdup( luaL_checkstring( L, -1 ) );
+                msg_Warn(p_mgr, "Extension capability '%s' unknown in"
+                         " script %s", psz_cap, psz_script);
             }
-            else
-            {
-                msg_Dbg( p_mgr, "In script %s, function descriptor() "
-                                "did not return a string as title.",
-                                psz_script );
-                p_ext->psz_title = strdup( psz_script );
-            }
-            lua_pop( L, 1 );
-
-            /* Get the fields from the extension manifest. */
-            p_ext->psz_author = GetStringFieldOrNull(L, "author");
-            p_ext->psz_description = GetStringFieldOrNull(L, "description");
-            p_ext->psz_shortdescription = GetStringFieldOrNull(L, "shortdesc");
-            p_ext->psz_url = GetStringFieldOrNull(L, "url");
-            p_ext->psz_version = GetStringFieldOrNull(L, "version");
-            /* Get icon data */
-            lua_getfield( L, -1, "icon" );
-            if( !lua_isnil( L, -1 ) && lua_isstring( L, -1 ) )
-            {
-                int len = lua_strlen( L, -1 );
-                p_ext->p_icondata = malloc( len );
-                if( p_ext->p_icondata )
-                {
-                    p_ext->i_icondata_size = len;
-                    memcpy( p_ext->p_icondata, lua_tostring( L, -1 ), len );
-                }
-            }
-            lua_pop( L, 1 );
-        }
-        else
-        {
-            msg_Warn( p_mgr, "In script %s, function descriptor() "
-                      "did not return a table!", psz_script );
-            goto exit;
+            /* Removes 'value'; keeps 'key' for next iteration */
+            lua_pop(L, 1);
         }
     }
     else
     {
-        msg_Err( p_mgr, "Script %s went completely foobar", psz_script );
-        goto exit;
+        msg_Warn(p_mgr, "In script %s, function descriptor() "
+                     "did not return a table of capabilities.",
+                     psz_script);
     }
+    lua_pop(L, 1);
 
-    msg_Dbg(p_mgr, "Script %s has the following capability flags: 0x%x",
-            psz_script, sys->i_capabilities);
-
-    b_ok = true;
-exit:
-    lua_close( L );
-    if( !b_ok )
+    /* Get title */
+    lua_getfield(L, -1, "title");
+    if (lua_isstring(L, -1))
     {
-        free( p_ext->psz_name );
-        free( p_ext->psz_title );
-        free( p_ext->psz_url );
-        free( p_ext->psz_author );
-        free( p_ext->psz_description );
-        free( p_ext->psz_shortdescription );
-        free( p_ext->psz_version );
-        free(sys);
-        free( p_ext );
+        p_ext->psz_title = strdup(luaL_checkstring(L, -1));
     }
     else
     {
-        /* Add the extension to the list of known extensions */
-        ARRAY_APPEND( p_mgr->extensions, p_ext );
+        msg_Dbg(p_mgr,"In script %s, function descriptor() "
+                      "did not return a string as title.",
+                      psz_script);
+        p_ext->psz_title = strdup(psz_script);
     }
+    lua_pop(L, 1);
+
+    /* Get the fields from the extension manifest. */
+    p_ext->psz_author = GetStringFieldOrNull(L, "author");
+    p_ext->psz_description = GetStringFieldOrNull(L, "description");
+    p_ext->psz_shortdescription = GetStringFieldOrNull(L, "shortdesc");
+    p_ext->psz_url = GetStringFieldOrNull(L, "url");
+    p_ext->psz_version = GetStringFieldOrNull(L, "version");
+    /* Get icon data */
+    lua_getfield(L, -1, "icon");
+    if (!lua_isnil(L, -1) && lua_isstring(L, -1))
+    {
+        int len = lua_strlen(L, -1);
+        p_ext->p_icondata = malloc(len);
+        if (p_ext->p_icondata)
+        {
+            p_ext->i_icondata_size = len;
+            memcpy(p_ext->p_icondata, lua_tostring(L, -1), len);
+        }
+    }
+    lua_pop(L, 1);
+
+
+    msg_Dbg(p_mgr, "Script %s has the following capability flags: 0x%x",
+            psz_script, sys->i_capabilities);
+
+    lua_close( L );
+    /* Add the extension to the list of known extensions */
+    ARRAY_APPEND(p_mgr->extensions, p_ext);
 
     /* Continue batch execution */
     return VLC_EGENERIC;
+discard:
+    lua_close(L);
+    free(p_ext->psz_name);
+    free(p_ext->psz_title);
+    free(p_ext->psz_url);
+    free(p_ext->psz_author);
+    free(p_ext->psz_description);
+    free(p_ext->psz_shortdescription);
+    free(p_ext->psz_version);
+    free(p_ext);
+    free(sys);
+    return VLC_EGENERIC;
 }
 
 static int Control(extensions_manager_t *p_mgr, int i_control,
@@ -672,46 +663,42 @@ static int GetMenuEntries( extensions_manager_t *p_mgr, extension_t *p_ext,
         goto exit;
     }
 
-    if( lua_gettop( L ) )
+    if (lua_gettop(L) == 0)
     {
-        if( lua_istable( L, -1 ) )
-        {
-            /* Get table size */
-            size_t i_size = lua_objlen( L, -1 );
-            *pppsz_titles = ( char** ) calloc( i_size+1, sizeof( char* ) );
-            *ppi_ids = ( uint16_t* ) calloc( i_size+1, sizeof( uint16_t ) );
-
-            /* Walk table */
-            size_t i_idx = 0;
-            lua_pushnil( L );
-            while( lua_next( L, -2 ) != 0 )
-            {
-                assert( i_idx < i_size );
-                if( (!lua_isstring( L, -1 )) || (!lua_isnumber( L, -2 )) )
-                {
-                    msg_Warn( p_mgr, "In script %s, an entry in "
-                              "the menu table is invalid!", p_ext->psz_name );
-                    goto exit;
-                }
-                (*pppsz_titles)[ i_idx ] = strdup( luaL_checkstring( L, -1 ) );
-                (*ppi_ids)[ i_idx ] = luaL_checkinteger( L, -2 ) & 0xFFFF;
-                i_idx++;
-                lua_pop( L, 1 );
-            }
-        }
-        else
-        {
-            msg_Warn( p_mgr, "Function menu() in script %s "
-                      "did not return a table", p_ext->psz_name );
-            goto exit;
-        }
+        msg_Warn(p_mgr, "Script %s went completely foobar", p_ext->psz_name);
+        goto exit;
     }
-    else
+
+    if (!lua_istable(L, -1))
     {
-        msg_Warn( p_mgr, "Script %s went completely foobar", p_ext->psz_name );
+        msg_Warn(p_mgr, "Function menu() in script %s "
+                 "did not return a table", p_ext->psz_name);
         goto exit;
     }
 
+    /* Get table size */
+    size_t i_size = lua_objlen( L, -1 );
+    *pppsz_titles = calloc(i_size+1, sizeof(char*));
+    *ppi_ids = calloc(i_size+1, sizeof(uint16_t));
+
+    /* Walk table */
+    size_t i_idx = 0;
+    lua_pushnil(L);
+    while (lua_next(L, -2) != 0)
+    {
+        assert(i_idx < i_size);
+        if(!lua_isstring(L, -1) || !lua_isnumber(L, -2))
+        {
+            msg_Warn(p_mgr, "In script %s, an entry in "
+                     "the menu table is invalid!", p_ext->psz_name);
+            goto exit;
+        }
+        (*pppsz_titles)[i_idx] = strdup(luaL_checkstring(L, -1));
+        (*ppi_ids)[i_idx] = luaL_checkinteger(L, -2) & 0xFFFF;
+        i_idx++;
+        lua_pop(L, 1);
+    }
+
     i_ret = VLC_SUCCESS;
 
 exit:
@@ -730,90 +717,89 @@ static lua_State* GetLuaState( extensions_manager_t *p_mgr,
 {
     assert( p_ext != NULL );
     struct lua_extension *sys = p_ext->p_sys;
-    lua_State *L = sys->L;
 
-    if( !L )
+    if (sys->L != NULL)
+        return sys->L;
+
+    lua_State *L = luaL_newstate();
+    if (L == NULL)
     {
-        L = luaL_newstate();
-        if( !L )
-        {
-            msg_Err( p_mgr, "Could not create new Lua State" );
-            return NULL;
-        }
-        vlclua_set_this( L, p_mgr );
-        intf_thread_t *intf = (intf_thread_t *) vlc_object_parent(p_mgr);
-        vlc_playlist_t *playlist = vlc_intf_GetMainPlaylist(intf);
-        vlclua_set_playlist_internal(L, playlist);
-        vlclua_extension_set( L, p_ext );
-
-        luaL_openlibs( L );
-        luaL_register_namespace( L, "vlc", p_reg );
-        luaopen_msg( L );
-
-        /* Load more libraries */
-        luaopen_config( L );
-        luaopen_dialog( L, p_ext );
-        luaopen_input( L );
-        luaopen_msg( L );
-        if (vlclua_fd_init(L, &sys->dtable))
-        {
-            lua_close( L );
-            return NULL;
-        }
-        luaopen_object( L );
-        luaopen_osd( L );
-        luaopen_playlist( L );
-        luaopen_stream( L );
-        luaopen_strings( L );
-        luaopen_variables( L );
-        luaopen_video( L );
-        luaopen_vlm( L );
-        luaopen_volume( L );
-        luaopen_xml( L );
-        luaopen_vlcio( L );
-        luaopen_errno( L );
-        luaopen_rand( L );
-        luaopen_rd( L );
-        luaopen_ml( L );
+        msg_Err(p_mgr, "Could not create new Lua State");
+        return NULL;
+    }
+    vlclua_set_this(L, p_mgr);
+    intf_thread_t *intf = (intf_thread_t *)vlc_object_parent(p_mgr);
+    vlc_playlist_t *playlist = vlc_intf_GetMainPlaylist(intf);
+    vlclua_set_playlist_internal(L, playlist);
+    vlclua_extension_set(L, p_ext);
+
+    luaL_openlibs(L);
+    luaL_register_namespace(L, "vlc", p_reg);
+    luaopen_msg(L);
+
+    /* Load more libraries */
+    luaopen_config(L);
+    luaopen_dialog(L, p_ext);
+    luaopen_input(L);
+    luaopen_msg(L);
+    if (vlclua_fd_init(L, &sys->dtable))
+    {
+        lua_close(L);
+        return NULL;
+    }
+    luaopen_object(L);
+    luaopen_osd(L);
+    luaopen_playlist(L);
+    luaopen_stream(L);
+    luaopen_strings(L);
+    luaopen_variables(L);
+    luaopen_video(L);
+    luaopen_vlm(L);
+    luaopen_volume(L);
+    luaopen_xml(L);
+    luaopen_vlcio(L);
+    luaopen_errno(L);
+    luaopen_rand(L);
+    luaopen_rd(L);
+    luaopen_ml(L);
 #if defined(_WIN32) && !defined(VLC_WINSTORE_APP)
-        luaopen_win( L );
+    luaopen_win(L);
 #endif
 
-        /* Register extension specific functions */
-        lua_getglobal( L, "vlc" );
-        lua_pushcfunction( L, vlclua_extension_deactivate );
-        lua_setfield( L, -2, "deactivate" );
-        lua_pushcfunction( L, vlclua_extension_keep_alive );
-        lua_setfield( L, -2, "keep_alive" );
-
-        /* Setup the module search path */
-        if( !strncmp( p_ext->psz_name, "zip://", 6 ) )
-        {
-            /* Load all required modules manually */
-            lua_register( L, "require", &vlclua_extension_require );
-        }
-        else if (vlclua_add_modules_path(L, p_ext->psz_name))
-        {
-            msg_Warn(p_mgr, "Error while setting the module "
-                     "search path for %s", p_ext->psz_name);
-            vlclua_fd_cleanup(&sys->dtable);
-            lua_close(L);
-            return NULL;
-        }
+    /* Register extension specific functions */
+    lua_getglobal(L, "vlc");
+    lua_pushcfunction(L, vlclua_extension_deactivate);
+    lua_setfield(L, -2, "deactivate");
+    lua_pushcfunction(L, vlclua_extension_keep_alive);
+    lua_setfield(L, -2, "keep_alive");
 
-        /* Load and run the script(s) */
-        if( vlclua_dofile( VLC_OBJECT( p_mgr ), L, p_ext->psz_name ) )
-        {
-            msg_Warn( p_mgr, "Error loading script %s: %s", p_ext->psz_name,
-                      lua_tostring( L, lua_gettop( L ) ) );
-            vlclua_fd_cleanup(&sys->dtable);
-            lua_close( L );
-            return NULL;
-        }
+    /* Setup the module search path */
+    if (!strncmp(p_ext->psz_name, "zip://", 6))
+    {
+        /* Load all required modules manually */
+        lua_register(L, "require", &vlclua_extension_require);
+    }
+    else if (vlclua_add_modules_path(L, p_ext->psz_name))
+    {
+        msg_Warn(p_mgr, "Error while setting the module "
+                "search path for %s", p_ext->psz_name);
+        vlclua_fd_cleanup(&sys->dtable);
+        lua_close(L);
+        return NULL;
+    }
 
-        sys->L = L;
+    /* Load and run the script(s) */
+    if (vlclua_dofile(VLC_OBJECT(p_mgr), L, p_ext->psz_name))
+    {
+        msg_Warn(p_mgr, "Error loading script %s: %s", p_ext->psz_name,
+                 lua_tostring(L, lua_gettop(L)));
+        vlclua_fd_cleanup(&sys->dtable);
+        lua_close(L);
+        return NULL;
     }
 
+    sys->L = L;
+
     return L;
 }
 



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/daac9a10cb3cf98697dda96a334610694a470ea2...fe1c31bcd2e4988fd7faabad4c0d07f5ce39f621

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/daac9a10cb3cf98697dda96a334610694a470ea2...fe1c31bcd2e4988fd7faabad4c0d07f5ce39f621
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list