hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Patrick McCabe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-7147) setnetgrent in native code is not portable
Date Thu, 28 Jun 2012 05:02:43 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-7147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402848#comment-13402848

Colin Patrick McCabe commented on HADOOP-7147:

>  int maxposix=sysconf(_SC_NGROUPS_MAX);
>  gid_t *gps = (gid_t *) malloc(maxposix * sizeof (gid_t));
>  if (!gps) {
>    *ngroups = 0;
>    free(pwbuf);
>    return ENOMEM;
>   }

Allocating an array of size _SC_NGROUPS_MAX seems a little excessive.  I think it would be
better to use realloc() to expand an array of gid_t as needed.

  // set the name of the group for subsequent calls to getnetgrent
  // note that we want to end group lokup regardless whether setnetgrent
  // was successful or not (as long as it was called we need to call
  // endnetgrent)
  setnetgrentCalledFlag = 1;
  setnetgrent(cgroup); // some platforms return void here

  UserList *current = NULL;
  // three pointers are for host, user, domain, we only care
  // about user now
  char *p[3];
  while(getnetgrent(p, p + 1, p + 2)) {
    if(p[1]) {
      current = (UserList *)malloc(sizeof(UserList));
      current->string = malloc(strlen(p[1]) + 1);
      strcpy(current->string, p[1]);
      current->next = userListHead;
      userListHead = current;
  // shortcut null, nonexistent groups
  if (userListSize == 0) 
    goto END;

It seems like this should be extracted into a function which would just return a linked list
of UserList.  There's no reason why the JNI function itself needs to reference setgrent(),
endnetgrent(), etc.

I'm also disturbed by the non-re-entrancy of the getnetgrent() function.  It seems like if
we're going to use this function, we need to put a mutex around it.  Alternately, getnetgrent_r()
is re-entrant, and available on Linux (but not on some other platforms).  You could do exactly
the same thing you did here and add this to the CMakeLists.txt:

The uses of getnetgrent() need to be surrounded by a mutex, probably similar to this:
static pthread_mutex_t g_getnetgrent_lock = PTHREAD_MUTEX_INITIALIZER;

UserList *get_net_groups(const char *name) {
  ... do non-thread-safe stuff ...
> setnetgrent in native code is not portable
> ------------------------------------------
>                 Key: HADOOP-7147
>                 URL: https://issues.apache.org/jira/browse/HADOOP-7147
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: native
>    Affects Versions: 0.22.0, 0.23.0
>            Reporter: Todd Lipcon
>            Assignee: Allen Wittenauer
>         Attachments: HADOOP-7147.patch, hadoop-7147.patch
> HADOOP-6864 uses the setnetgrent function in a way which is not compatible with BSD APIs,
where the call returns void rather than int. This prevents the native libs from building on
OSX, for example.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira


View raw message