On Mon, May 21, 2018 at 3:04 PM, James Darnley <james.darn...@gmail.com> wrote: > 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.
Yes, artifact from early testing, changed and tested, pkg-config adds the appropriate include directories. > >> +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. I've been wondering about this, but didn't look into it too deeply. > > 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. This should actually not happen since it is tested against LUA_TTABLE before starting to read it. (All other variables that are extracted from the stack are checked for the corresponding type as well.) > > 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. Thanks, this sounds very reasonable and I have already started reading the documentation on how to make use of that. I have also locally fixed the other things addressed (AVERROR usage and alloc and thread creation checks), but I have yet to test it works properly. Thanks again! Stephan _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel