[
https://issues.apache.org/jira/browse/HADOOP-9439?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Colin Patrick McCabe updated HADOOP-9439:
-----------------------------------------
Attachment: HADOOP-9439.003.patch
bq. I don't get why empty is a parameter here... aside from it being a
premature optimization (is allocating an empty array that bad?), it seems like
the JNI code could just grab this field itself and return a reference,
rather than taking it as a parameter.
This optimization was in the existing code, so I kept it in. But actually,
looking at it again, I think you're right. We might as well take it out.
bq. Style nit: extra space here before getGroupsForUser
ok.
bq. Can we use strerror here on the return, assuming it's probably a standard
errnum?
good idea.
+ ret = hadoop_group_info_fetch(ginfo, uinfo->gids[i]);
+ if (!ret) {
bq. Here if the group info lookup fails for a particular gid, you end up
swallowing the error without any warnings, etc. Maybe instead we should just
return the numeric gid as a group? This is what the 'groups' shell command
does:
The 'groups' shell command returns nonzero if it can't look up a group (at
least according to the internet; I didn't want to mess up my groups settings to
test). That would cause ShellBasedUnixGroupsMapping to get "got exception
trying to get groups for user." So as far as I know, there is no scenario
currently where we treat the numeric gid as a group name.
We could be bug-compatible with ShellBasedUnixGroupsMapping, and refuse to look
up *any* groups if one fails. However, that doesn't seem like a good idea.
Instead, I added some code that fires off a log4j message in that scenario, and
continues with the other groups.
bq. We may have to actually share this lock and configuration with the lock
used in NativeIO.c.
yeah, let's do that.
bq. Additionally, when I did that patch, I found it simpler to continue to use
the reentrant functions wrapped with a lock – less repeated code, etc.
ok.
> JniBasedUnixGroupsMapping: fix some crash bugs
> ----------------------------------------------
>
> Key: HADOOP-9439
> URL: https://issues.apache.org/jira/browse/HADOOP-9439
> Project: Hadoop Common
> Issue Type: Bug
> Components: native
> Affects Versions: 2.0.4-alpha
> Reporter: Colin Patrick McCabe
> Assignee: Colin Patrick McCabe
> Priority: Minor
> Attachments: HADOOP-9439.001.patch, HADOOP-9439.003.patch,
> HDFS-4640.002.patch
>
>
> JniBasedUnixGroupsMapping has some issues.
> * sometimes on error paths variables are freed prior to being initialized
> * re-allocate buffers less frequently (can reuse the same buffer for multiple
> calls to getgrnam)
> * allow non-reentrant functions to be used, to work around client bugs
> * don't throw IOException from JNI functions if the JNI functions do not
> declare this checked exception.
> * don't bail out if only one group name among all the ones associated with a
> user can't be looked up.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira