> On Jun 11, 2019, at 12:15 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
>
> Hello Max,
>
> I believe $(call FindSrcDirsForLib, $(MODULE), sspi_bridge) for SRC is the
> default so you shouldn't need to specify it explicitly, or did you experience
> some problems with the default that I'm missing?
I don't remember. I must have copied it from somewhere. The only special thing
about this target is that it includes a header file from the shared area.
I am running make again to see if it's useless.
Thanks,
Max
>
> /Erik
>
> On 2019-06-09 19:30, Weijun Wang wrote:
>> Updated webrev at
>>
>> http://cr.openjdk.java.net/~weijun/6722928/webrev.08/
>>
>> @build-dev guys, I realize I've never included you in this code review.
>> Please take a look.
>>
>> @Valerie, All comments accepted except for one (see below). In fact, I think
>> I found a bug in gss_release_context that it might release a cred handle
>> passed in, so I add a isLocalCred flag. However, I test it on my local
>> Windows and it seems the same handle can be FreeCredentialsHandle and then
>> used and then freed again.
>>
>>> On Jun 7, 2019, at 10:45 AM, Valerie Peng <valerie.p...@oracle.com> wrote:
>>>
>>> Hi, Max,
>>>
>>> <gssapi.h>
>>>
>>> - line 424: the "(used to be const)" comment can now be removed.
>>>
>>> <sspi.cpp>
>>>
>>> - line 396-403: on line 338, there is no need to go to err as no memory has
>>> been allocated. What happens when jumping to err but the variables, i.e.
>>> value and name, have not been declared? Line 400-401 seems not used as
>>> there is no more goto err after line 391.
>>>
>>> - line 528: the size of buffer here is 4*len + 1, but then when calling
>>> WideCharToMultiByte, the 6th argument is len. Seems inconsistent? line 534:
>>> shouldn't we free "buffer" here?
>>>
>>> - line 596: free cred allocated on line 588? line 610 and 617: free cred
>>> and cred->phCredK? line 638 and 644, 648 and 653: free cred, cred->phCredK
>>> and cred->phCredS?
>>>
>>> - line 829: free the context handle allocated on line 807? line 879: free
>>> newCred? line 901: no memory de-allocation before returning error? line
>>> 921: seems redundant given line 918.
>>>
>>> - line 1071: based on gss api doc, context_handle should be set to
>>> GSS_C_NO_CONTEXT after deletion.
>>>
>>> - line 1333: what about secBuff[1].pvBuffer?
>> According to the DecryptMessage spec, "The encrypted message is decrypted in
>> place, overwriting the original contents of its buffer". I've printed out
>> both secBuff[0].pvBuffer and secBuff[1].pvBuffer after the decryption and
>> the latter is indeed inside the former.
>>
>>> - line 1390, 1393, 1397: call gss_release_oid_set before returning failure?
>>>
>>> - line 1471: should we return an error code here when FormatMessage() call
>>> failed?
>>>
>>> Rest looks fine.
>>>
>>> Thanks,
>>>
>>> Valerie
>> Thanks,
>> Max
>>
>> [1]
>> https://docs.microsoft.com/en-us/windows/desktop/api/sspi/nf-sspi-decryptmessage
>>
>>> On 6/4/2019 2:52 AM, Weijun Wang wrote:
>>>> I uploaded an updated webrev in place. The only changes are:
>>>>
>>>> 1. s/SSPI_TRACE/SSPI_BRIDGE_TRACE/ in sspi.cpp
>>>> 2. Several copyright year updates.
>>>> 3. Remove one unused import.
>>>>
>>>> Thanks,
>>>> Max
>>>>
>>>>> On May 30, 2019, at 11:18 AM, Weijun Wang <weijun.w...@oracle.com> wrote:
>>>>>
>>>>> Here is the latest webrev
>>>>>
>>>>> http://cr.openjdk.java.net/~weijun/6722928/webrev.07/
