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, [e-mail] 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
À : [e-mail]
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, [e-mail] 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
[e-mail]
http://www.reactos.org/mailman/listinfo/ros-dev