On 2016-03-24 16:35, Christian Ullrich wrote:
> * From: Robbie Harwood [mailto:rharwood@redhat.com]
>
>> Christian Ullrich <chris@chrullrich.net> writes:
>>> pg_SSPI_recvauth(Port *port)
>>> {
>>> int mtype;
>>> + int status;
>>
>> The section of this function for include_realm checking already uses an
>> int status return code (retval). I would expect to see them share a
>> variable rather than have both "retval" and "status".
>
> I would not, because retval is local to that last if, but you are right, status
> does not need to be in function scope.
Moved declaration.
>>> + /* Build SAM name (DOMAIN\\user), then translate to UPN
>>> + (user@kerberos.realm). The realm name is returned in
>>> + lower case, but that is fine because in SSPI auth,
>>> + string comparisons are always case-insensitive. */
>>
>> Since we're already considering changing things: this is not the comment
>> style for this file (though it is otherwise a good comment).
>
> True. Will fix.
Reformatted.
>>> + upname = (char*)palloc(upnamesize);
>>
>> I don't believe this cast is typically included.
>
> Left over from when this was malloc() before Magnus' first look at it.
Removed.
Updated patch attached.
--
Christian