Re: [ros-dev] [ros-diffs] [hbelusca] 74068: [WS2_32] - Fix pointers verification in WSALookupServiceNextW and WSALookupServiceNextA; - In WSALookupServiceNextA: Perform extra checks to see whether we failed the first local ...

[Thomas Faber]

On 2017-03-05 01:40, hbelusca(a)svn.reactos.org wrote:
>   +
>  +    /*
>  +     * Check whether we actually just retrieved the needed buffer size
>  +     * because our previous local allocation did fail. If so, allocate
>  +     * a new buffer and retry again.
>  +     */
>  +    if ( (!UnicodeQuerySet) && (*lpdwBufferLength >= sizeof(WSAQUERYSETW))
> &&
>  +         (ErrorCode == SOCKET_ERROR) && (GetLastError() == WSAEFAULT) ) 
Seems like you'd want to check WSAGetLastError when looking for WSA* 
error codes.

>   +    {
>  +        /* Allocate the buffer we'll use */
>  +        UnicodeQuerySet = HeapAlloc(WsSockHeap, 0, UnicodeQuerySetSize);
>  +        if (UnicodeQuerySet)
>  +        {
>  +            /* Call the Unicode Function */
>  +            ErrorCode = WSALookupServiceNextW(hLookup,
>  +                                              dwControlFlags,
>  +                                              &UnicodeQuerySetSize,
>  +                                              UnicodeQuerySet);
>  +        }
>  +        /*
>  +         * Otherwise the allocation failed and we
>  +         * fall back into the error checks below.
>  +         */
>  +    }
>  +
>       if (ErrorCode == ERROR_SUCCESS)
>       {
>  -        /* Not convert to ANSI */
>  +        /* Now convert back to ANSI */
>           ErrorCode = MapUnicodeQuerySetToAnsi(UnicodeQuerySet,
>                                                lpdwBufferLength,
>                                                lpqsResults);
>  -        if (ErrorCode != ERROR_SUCCESS) SetLastError(ErrorCode);
>  +        if (ErrorCode != ERROR_SUCCESS)
>  +            SetLastError(ErrorCode);
>       }
>       else
>       {
>  @@ -499,10 +542,11 @@
>       }
>
>       /* If we had a local buffer, free it */
>  -    if (UnicodeQuerySet) HeapFree(WsSockHeap, 0, UnicodeQuerySet);
>  +    if (UnicodeQuerySet)
>  +        HeapFree(WsSockHeap, 0, UnicodeQuerySet); 
HeapFree(NULL) is fine by the way.

>  
>       /* Return to caller */
>  -    return ErrorCode == ERROR_SUCCESS ? ErrorCode : SOCKET_ERROR;
>  +    return (ErrorCode == ERROR_SUCCESS) ? ErrorCode : SOCKET_ERROR;
>   }

[Hermès BÉLUSCA-MAÏTO]

Hi, thanks for your comments.
The rest of the ws2_32 code continues to use Set/GetLastError even for dealing with the
WSA* error codes, and the WSASet/GetLastError just call back the normal functions, so I
would just keep using GetLastError here.

Hermès

-----Message d'origine-----
De : Ros-dev [mailto:[e-mail]] De la part de Thomas Faber
Envoyé : dimanche 5 mars 2017 12:06
À : ros-dev(a)reactos.org
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 74068: [WS2_32] - Fix pointers verification
in WSALookupServiceNextW and WSALookupServiceNextA; - In WSALookupServiceNextA: Perform
extra checks to see whether we failed the first local ...

On 2017-03-05 01:40, hbelusca(a)svn.reactos.org wrote:
>   +
>  +    /*
>  +     * Check whether we actually just retrieved the needed buffer size
>  +     * because our previous local allocation did fail. If so, allocate
>  +     * a new buffer and retry again.
>  +     */
>  +    if ( (!UnicodeQuerySet) && (*lpdwBufferLength >= sizeof(WSAQUERYSETW))
> &&
>  +         (ErrorCode == SOCKET_ERROR) && (GetLastError() == WSAEFAULT) 
>  + ) 
Seems like you'd want to check WSAGetLastError when looking for WSA* error codes.

>   +    {
>  +        /* Allocate the buffer we'll use */
>  +        UnicodeQuerySet = HeapAlloc(WsSockHeap, 0, UnicodeQuerySetSize);
>  +        if (UnicodeQuerySet)
>  +        {
>  +            /* Call the Unicode Function */
>  +            ErrorCode = WSALookupServiceNextW(hLookup,
>  +                                              dwControlFlags,
>  +                                              &UnicodeQuerySetSize,
>  +                                              UnicodeQuerySet);
>  +        }
>  +        /*
>  +         * Otherwise the allocation failed and we
>  +         * fall back into the error checks below.
>  +         */
>  +    }
>  +
>       if (ErrorCode == ERROR_SUCCESS)
>       {
>  -        /* Not convert to ANSI */
>  +        /* Now convert back to ANSI */
>           ErrorCode = MapUnicodeQuerySetToAnsi(UnicodeQuerySet,
>                                                lpdwBufferLength,
>                                                lpqsResults);
>  -        if (ErrorCode != ERROR_SUCCESS) SetLastError(ErrorCode);
>  +        if (ErrorCode != ERROR_SUCCESS)
>  +            SetLastError(ErrorCode);
>       }
>       else
>       {
>  @@ -499,10 +542,11 @@
>       }
>
>       /* If we had a local buffer, free it */
>  -    if (UnicodeQuerySet) HeapFree(WsSockHeap, 0, UnicodeQuerySet);
>  +    if (UnicodeQuerySet)
>  +        HeapFree(WsSockHeap, 0, UnicodeQuerySet); 
HeapFree(NULL) is fine by the way.

>  
>       /* Return to caller */
>  -    return ErrorCode == ERROR_SUCCESS ? ErrorCode : SOCKET_ERROR;
>  +    return (ErrorCode == ERROR_SUCCESS) ? ErrorCode : SOCKET_ERROR;
>   } 

_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev