Hi, please see the updated attached patch. In terms of style, I've implemented your latter suggestion as it is more compact, but happy either way. Cheers, Rudolph On 14 June 2017 at 20:05, Rich Felker wrote: > On Sun, Jun 11, 2017 at 12:59:51PM -0400, Rudolph Pereira wrote: > > Hi Rich, > > > > thanks for the feedback. I've attached a patch that implements errno > > setting as you suggested, other than a couple of cases where the code > > immediately returns. This also brings it in line with existing code > > (in __getpw_a/__getgr_a) so makes things more consistent. Please see > > attached - note this is against HEAD. > > It looks like you omitted the change to getgr_r.c corresponding to > this one for getpw_r.c: > > > diff --git a/src/passwd/getpw_r.c b/src/passwd/getpw_r.c > > index e8cc811..0c87ab0 100644 > > --- a/src/passwd/getpw_r.c > > +++ b/src/passwd/getpw_r.c > > @@ -27,6 +27,7 @@ static int getpw_r(const char *name, uid_t uid, struct > passwd *pw, char *buf, si > > } > > free(line); > > pthread_setcancelstate(cs, 0); > > + if (rv) errno = rv; > > return rv; > > } > > Also: > > > diff --git a/src/passwd/getspnam_r.c b/src/passwd/getspnam_r.c > > index 9233952..47ce3d3 100644 > > --- a/src/passwd/getspnam_r.c > > +++ b/src/passwd/getspnam_r.c > > @@ -72,14 +72,24 @@ int getspnam_r(const char *name, struct spwd *sp, > char *buf, size_t size, struct > > > > /* Disallow potentially-malicious user names */ > > if (*name=='.' || strchr(name, '/') || !l) > > + { > > + errno = EINVAL; > > return EINVAL; > > + } > > Please use consistent style for braces (open brace on if line). > Alternatively (if you don't balk at the style; not sure if others will > like it), this works: > > - return EINVAL; > + return errno = EINVAL; > > Rich >