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

[Daniel D. Daugherty]

On 7/11/16 10:16 AM, Alan Burlison wrote:
> 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.

Let me know when you're ready and I'll upload a new webrev
for you... sorry this has gotten so messy...

Dan


>
>> 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,
>