RFR: JDK-8160997 Solaris: deprecated <pwd.h> and <gid.h> interfaces should be replaced

[Alan Burlison]

On 11/07/2016 16:02, Daniel D. Daugherty wrote:

> jdk/src/jdk.security.auth/solaris/native/libjaas/Solaris.c
>      So you added the &p parameter to the getpwuid_r(), but
>      then you never check or use the returned 'p'.
>
>      Other places that had both Solaris and non-Solaris variants
>      had code like this:
>
>      L479:         if (result == 0 && p != NULL &&
>      L480:             p->pw_name != NULL && *(p->pw_name) != '\0') {
>      L481:             name = JNU_NewStringPlatform(env, p->pw_name);
>      L482:         }
>
>      Of course, this Solaris specific code checked the returned
>      '&pwd' value because it didn't have an &p parameter. I don't
>      this is a problem, especially since I don't understand why
>      this function has both an '&pwd' param and an '&p' param,
>      but I figured I would raise the question.

Ah, the difference between the POSIX draft & final versions isn't just a 
new parameter, they also changed the return type - ugh. I think the not 
null/empty check for pw_name is overkill but a closer look a the manpage 
suggests that the check for a non-zero result should also be accompanied 
by a check for a non-null value in p:

        The getpwnam_r() and getpwuid_r() functions can return 0  in 
the  case
        where  the requested entry is not found. The application needs 
to check
        the return value and that the result pointer is non-null. 
Otherwise, an
        error value is returned to indicate the error.

So I think the code in the patch should be:

      if (getpwuid_r(getuid(), &pwd, pwd_buf, sizeof(pwd_buf), &p) != 0 
&& p != NULL &&
          getgroups(numSuppGroups, groups) != -1) {

The preceding memset() isn't necessary but does no harm.

And while I'm in there I notice that the size of pw_buf is hardcoded, it 
should be whatever is returned by sysconf(_SC_GETPW_R_SIZE_MAX). That's 
actually 1024 but to be future-proof it should really use sysconf() - 
I'll fix that as well, and check the other files for the same sort of 
issue, and then redo the webrev and the tests.

> jdk/src/jdk.security.auth/unix/native/libjaas/Unix.c
>      No comments.
>
> Thumbs up! I don't know what to think of the libjaas/Solaris.c
> code not using the possibly duplicate '&p' param.
>>
>> JPRT + the hotspot testset was clean.
>>
>
> If I remember correctly, you also ran a regular JPRT test job
> (without -testset hotspot). I believe that the regular job
> runs JDK focused testing, but I'd have to look at the JPRT
> job to be sure.

I'll need to rerun them anyway after addressing the above issues, if 
there are other ones you'd like me to run then just let me know.

Thanks,

-- 
Alan Burlison
--