[vlc-devel] [vlc-commits] Add lua console handling code for windows.

[Pierre Ynard]

> --- a/modules/lua/Modules.am
> +++ b/modules/lua/Modules.am
> @@ -37,4 +37,8 @@ SOURCES_lua = \
>  	libs/xml.c \
>  	$(NULL)
>  
> +if HAVE_WIN32
> +SOURCES_lua += libs/win.c
> +endif

This won't add the file on WinCE...

>              luaopen_vlm( L );
>              luaopen_volume( L );
>              luaopen_xml( L );
> +#ifdef WIN32
> +            luaopen_win( L );
> +#endif

... however this would try to call luaopen_win() on WinCE, resulting in
a build failure.

> +    int i_timeout = luaL_optint( L, 2, -1 );
>  
>      int i_ret;
>      do
> -        i_ret = poll( p_fds, i_fds, -1 );
> +        i_ret = poll( p_fds, i_fds, i_timeout );
>      while( i_ret == -1 );

This fails to adjust the timeout if poll() is interrupted. Also exposing
the timeout in the API favors proliferation or low-quality code. See
314c242ab041d515c1b2f1a82ee1a3fe4fe715a5
 
> +/* Based on modules/control/rc.c and include/vlc_interface.h */

I understand for rc.c, but:

> +    AllocConsole();
> +
> +    freopen( "CONOUT$", "w", stdout );
> +    freopen( "CONOUT$", "w", stderr );
> +    freopen( "CONIN$", "r", stdin );

This code is taken from a macro in vlc_interface.h. Couldn't you just
call the macro?

> +    fprintf(stdout, "Tadam!\n");
> +    fflush(stdout);

What is the flush for?

> -        local ret = vlc.net.poll( pollfds )
> +        local timeout = -1
> +        if vlc.win and listeners.stdio then
> +            timeout = 50
> +        end
> +        local ret = 0
> +        if not vlc.win or listeners.tcp then
> +            ret = vlc.net.poll( pollfds, timeout )
> +        end

I'm not sure to understand well, if there are a windows console and TCP
sockets, then what prevents bogus file descriptors 0 and 1 from still
being polled along with all the clients?

>      local function destructor( h )
> -        print "destructor"
>          for _,client in pairs(clients) do
> -            --client:send("Shutting down.")
> +            client:send("Shutting down.")

This would re-add a duplicate of this message. "Shutting down." is
already broadcast in the shutdown function. Choose one.

> -    -- FIXME: this is never called, client sockets are leaked
> [...]
> +              { -- metatable
> +                __gc = destructor,
> +                __metatable = "",
> +              })

This won't work because __gc only works with userdata variables. See
6addf026892913a7b2518485bc35d2f9385722d9

-- 
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."