[FFmpeg-devel] [PATCH 2/8] ffserver: Implement lua config file reader
James Darnley
james.darnley at gmail.com
Mon May 21 16:04:44 EEST 2018
On 2018-05-20 20:53, Stephan Holljes wrote:
> +#include <lua5.3/lua.h>
> +#include <lua5.3/lauxlib.h>
> +#include <lua5.3/lualib.h>
That's not portable. Lua headers are not in a subdirectory.
> +int configs_read(struct HTTPDConfig **configs, const char *filename)
> +{
> + int ret = 0;
> + int nb_configs = 0;
> + int nb_streams = 0;
> + int nb_formats = 0;
> + int i;
> + int index = 0;
> + const char *key, *error;
> + struct HTTPDConfig *parsed_configs = NULL;
> + struct HTTPDConfig *config;
> + struct StreamConfig *stream;
> + lua_State *L = luaL_newstate();
> + ret = luaL_loadfile(L, filename);
> + if (ret != 0) {
> + fprintf(stderr, "Unable to open config file: %s\n", lua_tostring(L, -1));
> + lua_close(L);
> + return -1;
> + }
> +
> + ret = lua_pcall(L, 0, 0, 0);
> +
> + if (ret != 0) {
> + fprintf(stderr, "Unable to read config file: %s\n", lua_tostring(L, -1));
> + lua_close(L);
> + return -1;
> + }
> + lua_getglobal(L, "settings");
> + if (lua_type(L, -1) != LUA_TTABLE) {
> + lua_pushstring(L, "Error \"settings\" is not a table");
> + goto fail;
> + }
> + lua_pushnil(L);
> +
> + // iterate servers
> + while (lua_next(L, -2) != 0) {
> + nb_configs++;
> + parsed_configs = av_realloc(parsed_configs, nb_configs * sizeof(struct HTTPDConfig));
> + config = &parsed_configs[nb_configs - 1];
> + config->server_name = NULL;
> + config->bind_address = NULL;
> + config->port = 0;
> + config->accept_timeout = 1000;
> + config->streams = NULL;
> + config->nb_streams = 0;
> + if (lua_type(L, -2) != LUA_TSTRING) {
> + lua_pushstring(L, "Error server name is not a string.");
> + goto fail;
> + }
> + if (lua_type(L, -1) != LUA_TTABLE) {
> + lua_pushstring(L, "Error server settings is not a table.");
> + goto fail;
> + }
> + config->server_name = av_strdup(lua_tostring(L, -2));
> + lua_pushnil(L);
> + // iterate server properties
> + nb_streams = 0;
> + while(lua_next(L, -2) != 0) {
> + if (lua_type(L, -2) != LUA_TSTRING) {
> + lua_pushstring(L, "Error server property is not a string.");
> + goto fail;
> + }
> + key = lua_tostring(L, -2);
> + if (!strncmp("bind_address", key, 12)) {
> + config->bind_address = av_strdup(lua_tostring(L, -1));
> + } else if (!strncmp("port", key, 4)) {
> + config->port = (int) lua_tonumber(L, -1);
> + } else {
> + // keys that are not "bind_address" or "port" are streams
> + if (lua_type(L, -1) != LUA_TTABLE) {
> + lua_pushstring(L, "Error Stream configuration is not a table.");
> + goto fail;
> + }
> +
> + nb_streams++;
> + config->streams = av_realloc(config->streams, nb_streams * sizeof(struct StreamConfig));
> + stream = &config->streams[nb_streams - 1];
> + stream->input_uri = NULL;
> + stream->formats = NULL;
> + stream->stream_name = av_strdup(lua_tostring(L, -2));
> + lua_pushnil(L);
> + while(lua_next(L, -2) != 0) {
> + if (lua_type(L, -2) != LUA_TSTRING) {
> + lua_pushstring(L, "Error stream property is not a string.");
> + goto fail;
> + }
> + key = lua_tostring(L, -2);
> + if (!strncmp("input", key, 5)) {
> + stream->input_uri = av_strdup(lua_tostring(L, -1));
> + } else if (!strncmp("formats", key, 7)) {
> + index = 1;
> + nb_formats = 0;
> + lua_pushnumber(L, index);
> + while(1) {
> + lua_gettable(L, -2);
> + if (lua_isnil(L, -1))
> + break;
> + if (lua_type(L, -1) != LUA_TSTRING) {
> + lua_pushstring(L, "Error format name is not a string.");
> + goto fail;
> + }
> + stream->formats = av_realloc(stream->formats,
> + (nb_formats + 1) * sizeof(enum StreamFormat));
> + key = lua_tostring(L, -1);
> + if (!strncmp("mkv", key, 3)) {
> + stream->formats[nb_formats++] = FMT_MATROSKA;
> + } else {
> + fprintf(stderr, "Warning unknown format (%s) in stream format configuration.\n",
> + key);
> + av_realloc(stream->formats, nb_formats * sizeof(enum StreamFormat));
> + }
> + stream->nb_formats = nb_formats;
> + lua_pop(L, 1);
> + lua_pushnumber(L, ++index);
> + }
> + lua_pop(L, 1);
> +
> + } else {
> + fprintf(stderr, "Warning unknown key (%s) in stream configuration.\n", key);
> + }
> + lua_pop(L, 1);
> + }
> + }
> + lua_pop(L, 1);
> + }
> + config->nb_streams = nb_streams;
> + lua_pop(L, 1);
> + }
> +
> + lua_close(L);
> + *configs = parsed_configs;
> + return nb_configs;
> +
> +fail:
> + error = lua_tostring(L, -1);
> + fprintf(stderr, "%s\n", error);
> + lua_close(L);
> + for (i = 0; i < nb_configs; i++)
> + config_free(&parsed_configs[i]);
> + av_free(parsed_configs);
> + return -1;
> +}
Do you know what Lua does if any of these function calls raises an
error? I see you catching the error when reading the config file but
all other use of Lua is not in a protected environment.
Lua 5.3 manual, section 4.6:
> If an error happens outside any protected environment, Lua calls a panic function (see lua_atpanic) and then calls abort, thus exiting the host application.
Even worse: if someone sets the global variable "settings" to something
that isn't a table then lua_next will immediately segfault and you will
get no diagnostics. I was surprised by this because I thought it would
raise an error.
Other than these significant issues it looks like a reasonable use of
the Lua API.
My suggestions:
1 - Move all the Lua use into a lua_Cfunction and pcall it.
2 - Use that to raise a Lua error when checking types.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 603 bytes
Desc: OpenPGP digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180521/d52f2615/attachment.sig>
More information about the ffmpeg-devel
mailing list