From: Alexander Turenko <alexander.turenko@tarantool.org> To: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: tarantool-patches@freelists.org Subject: Re: [PATCH 2/2] lua: fix error handling in getpwall and getgrall Date: Mon, 3 Dec 2018 18:19:54 +0300 [thread overview] Message-ID: <20181203151953.qdhdx5d4zkqj5d2n@tkn_work_nb> (raw) In-Reply-To: <20181203083324.bjl6do7mvydxobif@esperanza> 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 <alexander.turenko@tarantool.org> 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
next prev parent reply other threads:[~2018-12-03 15:19 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-02 15:57 [PATCH 0/2] Glibc 2.28 fixes Alexander Turenko 2018-12-02 15:57 ` [PATCH 1/2] Remove deprecated getaddrinfo() flags Alexander Turenko 2018-12-02 15:57 ` [PATCH 2/2] lua: fix error handling in getpwall and getgrall Alexander Turenko 2018-12-03 8:33 ` Vladimir Davydov 2018-12-03 15:19 ` Alexander Turenko [this message] 2018-12-03 15:29 ` Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20181203151953.qdhdx5d4zkqj5d2n@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH 2/2] lua: fix error handling in getpwall and getgrall' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox