From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 0D42221AEE for ; Wed, 31 Jul 2019 13:13:05 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2mjTCPv119TT for ; Wed, 31 Jul 2019 13:13:04 -0400 (EDT) Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id BECD720A80 for ; Wed, 31 Jul 2019 13:13:04 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/2] lua: new function luaT_return_error() References: From: Vladislav Shpilevoy Message-ID: <2d0ba195-f5cb-bd23-6340-1426b506c82f@tarantool.org> Date: Wed, 31 Jul 2019 19:15:13 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, imeevma@tarantool.org Cc: kostja@tarantool.org Hi! Thanks for the patch! > diff --git a/src/lua/fio.c b/src/lua/fio.c > index 55fa667..cb7e31e 100644 > --- a/src/lua/fio.c > +++ b/src/lua/fio.c > @@ -47,11 +47,9 @@ > #include "lua/utils.h" > #include "coio_file.h" > > -static inline void > -lbox_fio_pushsyserror(struct lua_State *L) > -{ > - diag_set(SystemError, "fio: %s", strerror(errno)); > - luaT_pusherror(L, diag_get()->last); > +#define lbox_fio_pushsyserror(L) { \ > + diag_set(SystemError, "fio: %s", strerror(errno)); \ > + return luaT_return_error(L); \ > } Please, don't return here. It makes the code below confusing, when you call 'pushsyserror, pushinteger', and return 1. Make this macro return value of the last block, and surround it with '()' or with 'do {...} while(false)' to be force ';' after it, and be able to use it in 'if ... else ...' constructions. It is a standard practice. #define lbox_fio_pushsyserror(L) ({ \ diag_set(SystemError, "fio: %s", strerror(errno)); \ luaT_return_error(L); \ }) and use 'return lbox_fio_pushsyserror(L);' in the code below. > > static int > @@ -69,11 +67,8 @@ usage: > int mode = lua_tointeger(L, 3); > > int fh = coio_file_open(pathname, flags, mode); > - if (fh < 0) { > - lua_pushnil(L); > + if (fh < 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_pushinteger(L, fh); > return 1; > } > @@ -90,11 +85,8 @@ lbox_fio_pwrite(struct lua_State *L) > size_t offset = lua_tonumber(L, 4); > > int res = coio_pwrite(fh, buf, len, offset); > - if (res < 0) { > - lua_pushnil(L); > + if (res < 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_pushinteger(L, res); > return 1; > } > @@ -115,11 +107,8 @@ lbox_fio_pread(struct lua_State *L) > > int res = coio_pread(fh, buf, len, offset); > > - if (res < 0) { > - lua_pushnil(L); > + if (res < 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_pushinteger(L, res); > return 1; > } > @@ -129,7 +118,10 @@ lbox_fio_pushbool(struct lua_State *L, bool res) > { > lua_pushboolean(L, res); > if (!res) { > - lbox_fio_pushsyserror(L); > + diag_set(SystemError, "fio: %s", strerror(errno)); > + struct error *e = diag_last_error(diag_get()); > + assert(e != NULL); > + luaT_pusherror(L, e); > return 2; > } > return 1; > @@ -211,11 +203,8 @@ lbox_fio_write(struct lua_State *L) > size_t len = lua_tonumber(L, 3); > > int res = coio_write(fh, buf, len); > - if (res < 0) { > - lua_pushnil(L); > + if (res < 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_pushinteger(L, res); > return 1; > } > @@ -298,11 +287,8 @@ lbox_fio_read(struct lua_State *L) > > int res = coio_read(fh, buf, len); > > - if (res < 0) { > - lua_pushnil(L); > + if (res < 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_pushinteger(L, res); > return 1; > } > @@ -371,11 +357,8 @@ DEF_STAT_METHOD(is_sock, S_ISSOCK); > static int > lbox_fio_pushstat(struct lua_State *L, int res, const struct stat *stat) > { > - if (res < 0) { > - lua_pushnil(L); > + if (res < 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_newtable(L); > > PUSHTABLE("dev", lua_pushinteger, stat->st_dev); > @@ -518,11 +501,8 @@ lbox_fio_listdir(struct lua_State *L) > } > pathname = lua_tostring(L, 1); > char *buf; > - if (coio_readdir(pathname, &buf) < 0) { > - lua_pushnil(L); > + if (coio_readdir(pathname, &buf) < 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_pushstring(L, buf); > free(buf); > return 1; > @@ -613,11 +593,8 @@ usage: > goto usage; > char *path = (char *)lua_newuserdata(L, PATH_MAX); > int res = coio_readlink(pathname, path, PATH_MAX); > - if (res < 0) { > - lua_pushnil(L); > + if (res < 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_pushlstring(L, path, res); > lua_remove(L, -2); > return 1; > @@ -629,16 +606,11 @@ lbox_fio_tempdir(struct lua_State *L) > char *buf = (char *)lua_newuserdata(L, PATH_MAX); > if (!buf) { > errno = ENOMEM; > - lua_pushnil(L); > lbox_fio_pushsyserror(L); > - return 2; > } > > - if (coio_tempdir(buf, PATH_MAX) != 0) { > - lua_pushnil(L); > + if (coio_tempdir(buf, PATH_MAX) != 0) > lbox_fio_pushsyserror(L); > - return 2; > - } > lua_pushstring(L, buf); > lua_remove(L, -2); > return 1; > @@ -650,20 +622,12 @@ lbox_fio_cwd(struct lua_State *L) > char *buf = (char *)lua_newuserdata(L, PATH_MAX); > if (!buf) { > errno = ENOMEM; > - lua_pushnil(L); > lbox_fio_pushsyserror(L); > - return 2; > } > - > - > - if (getcwd(buf, PATH_MAX)) { > - lua_pushstring(L, buf); > - lua_remove(L, -2); > - } else { > + if (getcwd(buf, PATH_MAX) == NULL) > lbox_fio_pushsyserror(L); > - lua_pushnil(L); > - return 2; > - } > + lua_pushstring(L, buf); > + lua_remove(L, -2); > return 1; > }