Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2237)

Issue 101310044: os/user: group lookup functions

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 10 months ago by andre
Modified:
9 years, 6 months ago
Reviewers:
tegehel
Visibility:
Public.

Description

os/user: group lookup functions Adding group lookup functions analogous to the user lookup ones to the os/user package.

Patch Set 1 #

Patch Set 2 : diff -r 7d2e78c502ab https://code.google.com/p/go #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+272 lines, -22 lines) Patch
M src/pkg/os/user/lookup.go View 1 1 chunk +24 lines, -2 lines 0 comments Download
M src/pkg/os/user/lookup_stubs.go View 1 1 chunk +10 lines, -2 lines 0 comments Download
M src/pkg/os/user/lookup_unix.go View 1 2 chunks +182 lines, -18 lines 1 comment Download
M src/pkg/os/user/user.go View 1 2 chunks +25 lines, -0 lines 0 comments Download
M src/pkg/os/user/user_test.go View 1 1 chunk +31 lines, -0 lines 0 comments Download

Messages

Total messages: 1
tegehel
9 years, 6 months ago (2014-10-28 13:41:15 UTC) #1
Here are my 2 cents. It's not a complete review, just a quick bugfix for the
"userInGroup" function, because I've reused it myself in a project.

https://codereview.appspot.com/101310044/diff/20001/src/pkg/os/user/lookup_un...
File src/pkg/os/user/lookup_unix.go (right):

https://codereview.appspot.com/101310044/diff/20001/src/pkg/os/user/lookup_un...
src/pkg/os/user/lookup_unix.go:199: gid, err := strconv.Atoi(g.Gid)
I think you rather want u.Gid here.  The getgrouplist(3) manpage says:

       If it was not among the groups defined for user in the group
       database, then group is included in the list of groups returned by
       getgrouplist(); typically this argument is specified as the group ID
       from the password record for user.

Currently, with g.Gid here, the group we want to check is always added to the
resulting list from mygetgrouplist, meaning userInGroup(u,g) always returns
true...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b