Re: [2/3] winhttp: Implement IWinHttpRequest::get_ResponseBody.

[Jacek Caban]

Hi Hans,

On 07/22/11 16:00, Hans Leidekker wrote:
> ---
>  dlls/winhttp/request.c       |   13 +++++++++++--
>  dlls/winhttp/tests/winhttp.c |    9 ++++++++-
>  2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c
> index 0c4c726..a73be22 100644
> --- a/dlls/winhttp/request.c
> +++ b/dlls/winhttp/request.c
> @@ -2747,8 +2747,17 @@ static HRESULT WINAPI winhttp_request_get_ResponseBody(
>      IWinHttpRequest *iface,
>      VARIANT *body )
>  {
> -    FIXME("\n");
> -    return E_NOTIMPL;
> +    struct winhttp_request *request = impl_from_IWinHttpRequest( iface );
> +    DWORD err;
> +
> +    TRACE("%p, %p\n", request, body);
> +
> +    if ((err = request_read_body( request, INFINITE ))) return HRESULT_FROM_WIN32( err );
> +
> +    VariantInit( body );
> +    V_VT( body ) = VT_ARRAY|VT_UI1;
> +    V_ARRAY( body ) = request->buffer;

The caller is responsible for freeing the result, so it will destroy
your internal data. I think you should create an array here and not use
it to store the data internally.

Also VariantInit call is not needed if you set both type and value of
variant.


Jacek

[2/3] winhttp: Implement IWinHttpRequest::get_ResponseBody.

[Hans Leidekker]

On Fri, 2011-07-22 at 16:16 +0200, Jacek Caban wrote:
> The caller is responsible for freeing the result, so it will destroy
> your internal data. I think you should create an array here and not use
> it to store the data internally.

How do you know that the caller is responsible for freeing this
property? It seems wasteful to duplicate this potentially large buffer.

[2/3] winhttp: Implement IWinHttpRequest::get_ResponseBody.

[Jacek Caban]

On 07/22/11 16:32, Hans Leidekker wrote:
> On Fri, 2011-07-22 at 16:16 +0200, Jacek Caban wrote:
> > The caller is responsible for freeing the result, so it will destroy
> > your internal data. I think you should create an array here and not use
> > it to store the data internally.
> > How do you know that the caller is responsible for freeing this
> > property?

That's how out and retval values work in OLE automation, you wouldn't be
able to control their life time otherwise. As a test, you may add
VariantClear call in your tests and see how things will work.
> It seems wasteful to duplicate this potentially large buffer.

I agree, but we can't redesign the API.

Jacek