From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 3 Dec 2018 18:19:54 +0300 From: Alexander Turenko Subject: Re: [PATCH 2/2] lua: fix error handling in getpwall and getgrall Message-ID: <20181203151953.qdhdx5d4zkqj5d2n@tkn_work_nb> References: <41e2e4cf352718091a8f5c3b7b09537ea4723dd3.1543765908.git.alexander.turenko@tarantool.org> <20181203083324.bjl6do7mvydxobif@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20181203083324.bjl6do7mvydxobif@esperanza> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: We had discussion with Vladimir and decided to remove errno checks when a return value does not indicate possibility of an error. The new patch is at bottom of the email. WBR, Alexander Turenko. On Mon, Dec 03, 2018 at 11:33:24AM +0300, Vladimir Davydov wrote: > On Sun, Dec 02, 2018 at 06:57:32PM +0300, Alexander Turenko wrote: > > This commit fixes app-tap/pwd.test.lua test. It seems that the problem > > appears after updating to glibc-2.28. > > > > Related to #3766. > > --- > > src/lua/pwd.lua | 43 +++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 39 insertions(+), 4 deletions(-) > > > > diff --git a/src/lua/pwd.lua b/src/lua/pwd.lua > > index 8f17951df..0a6b73395 100644 > > --- a/src/lua/pwd.lua > > +++ b/src/lua/pwd.lua > > @@ -158,22 +158,52 @@ local function getpw(uid) > > return user > > end > > > > +-- It seems glibc developers threat POSIX in the following way. > > s/threat/treat > Removed the comment at all. > > +-- {set,get,end}pwent() and {set,get,end}grent() functions can set > > +-- errno to non-zero value that is not listed in the standard in > > +-- case of success. Errno should be checked after get{pw,gr}ent > > +-- only when it returns a non-NULL value. > > Why do you need to check for specific error codes then? > Discussed it with Vladimir and decided to check only get{pw,gr}ent() only when they return NULL. > > +-- > > +-- https://sourceware.org/bugzilla/show_bug.cgi?id=1969 > > +-- https://sourceware.org/bugzilla/show_bug.cgi?id=23737 > > + > > +local pwent_grent_errno_list = { > > + errno.EINTR, > > + errno.EIO, > > + errno.EMFILE, > > + errno.ENFILE, > > +} > > + > > +local function is_pwent_grent_errno(e) > > + for _, v in ipairs(pwent_grent_errno_list) do > > + if e == v then > > + return true > > + end > > + end > > + return false > > +end > > + > > local function getpwall() > > errno(0) > > ffi.C.setpwent() > > - if errno() ~= 0 then > > + if is_pwent_grent_errno(errno()) then > > According to the manual setpwent never fails in any visible to the user > way so why check errno here? > It does not stated explicitly in manual pages or POSIX, but it seems to be in spirit of Unix APIs. > > return nil > > end > > local pws = {} > > while true do > > + errno(0) > > local pw = ffi.C.getpwent() > > if pw == nil then > > + if is_pwent_grent_errno(errno()) then > > + return nil > > + end > > break > > end > > table.insert(pws, getpw(pw.pw_uid)) > > end > > + errno(0) > > ffi.C.endpwent() > > - if errno() ~= 0 then > > + if is_pwent_grent_errno(errno()) then > > Again, endpwent never fails. May be, remove this check altogether? > > Same concerns setgrent and endgrent. Done. > > > return nil > > end > > return pws > > @@ -182,19 +212,24 @@ end > > local function getgrall() > > errno(0) > > ffi.C.setgrent() > > - if errno() ~= 0 then > > + if is_pwent_grent_errno(errno()) then > > return nil > > end > > local grs = {} > > while true do > > + errno(0) > > local gr = ffi.C.getgrent() > > if gr == nil then > > + if is_pwent_grent_errno(errno()) then > > + return nil > > + end > > break > > end > > table.insert(grs, getpw(gr.gr_gid)) > > end > > + errno(0) > > ffi.C.endgrent() > > - if errno() ~= 0 then > > + if is_pwent_grent_errno(errno()) then > > return nil > > end > > return grs The new patch ------------- commit d8a5e29ecfd486a3f998d4a41d8e0bb17463c27d Author: Alexander Turenko Date: Sun Dec 2 17:58:09 2018 +0300 lua: fix error handling in getpwall and getgrall This commit fixes app-tap/pwd.test.lua test. It seems that the problem appears after updating to glibc-2.28. It seems that usual way to handle errors in Unix is to check errno only when a return value indicates possibility of an error. Related to #3766. diff --git a/src/lua/pwd.lua b/src/lua/pwd.lua index 8f17951df..b2c8e121f 100644 --- a/src/lua/pwd.lua +++ b/src/lua/pwd.lua @@ -159,44 +159,37 @@ local function getpw(uid) end local function getpwall() - errno(0) ffi.C.setpwent() - if errno() ~= 0 then - return nil - end local pws = {} while true do + errno(0) local pw = ffi.C.getpwent() if pw == nil then + if errno() ~= 0 then + return nil + end break end table.insert(pws, getpw(pw.pw_uid)) end ffi.C.endpwent() - if errno() ~= 0 then - return nil - end return pws end local function getgrall() - errno(0) ffi.C.setgrent() - if errno() ~= 0 then - return nil - end local grs = {} while true do local gr = ffi.C.getgrent() if gr == nil then + if errno() ~= 0 then + return nil + end break end table.insert(grs, getpw(gr.gr_gid)) end ffi.C.endgrent() - if errno() ~= 0 then - return nil - end return grs end