Error handling in RpcBindingVectorFree (dlls/rpcrt4/rpc_binding.c)

[Gerald Pfeifer]

Looking at 

  RPC_STATUS WINAPI RpcBindingVectorFree( RPC_BINDING_VECTOR** BindingVector )
  {
    RPC_STATUS status;
    ULONG c;

    TRACE("(%p)\n", BindingVector);
    for (c=0; c<(*BindingVector)->Count; c++) {
      status = RpcBindingFree(&(*BindingVector)->BindingH[c]);
    }
    HeapFree(GetProcessHeap(), 0, *BindingVector);
    *BindingVector = NULL;
    return RPC_S_OK;
  }

we currently always ignore the outcome of RpcBindingFree and return 
RPC_S_OK.

However, there is one case where RpcBindingFree returns something 
different (which is if *Binding is null when RPC_S_INVALID_BINDING
is returned).

What is the proper way of handling this?  Just keeping the code as
is and removing the unused status variable?  Breaking the loop once
RpcBindingFree returns something different from RPC_S_OK?  Continuing
and returning the first / the last status different from RPC_S_OK?

Gerald

[Gerald Pfeifer]

I did not see a response to my question back then, but a few months
this was addressed by the following patch:

  commit 95ea3527385aed7d9f0e0dc7fce755407d28acf5
  Author: Alexandre Julliard <julliard(a)winehq.org>
  Date:   Wed Sep 1 12:39:01 2010 +0200

    rpcrt4: Remove a couple of unused local variables.

Theoretically RpcBindingFree in the loop can still return 
RPC_S_INVALID_BINDING if provided with the NULL pointer, but I 
guess this won't be the case for any RPC_BINDING_VECTOR** every 
passed to RpcBindingVectorFree() ?

Gerald

PS: Yes, I'm going through some older submissions of mine. ;-)

On Sun, 9 May 2010, Gerald Pfeifer wrote:
> Looking at
>
> RPC_STATUS WINAPI RpcBindingVectorFree( RPC_BINDING_VECTOR** BindingVector )
>   {
>     RPC_STATUS status;
>     ULONG c;
>
> TRACE("(%p)\n", BindingVector);
>     for (c=0; c<(*BindingVector)->Count; c++) {
>       status = RpcBindingFree(&(*BindingVector)->BindingH[c]);
>     }
>     HeapFree(GetProcessHeap(), 0, *BindingVector);
>     *BindingVector = NULL;
>     return RPC_S_OK;
>   }
>
> we currently always ignore the outcome of RpcBindingFree and return 
> RPC_S_OK.
>
> However, there is one case where RpcBindingFree returns something 
> different (which is if *Binding is null when RPC_S_INVALID_BINDING
> is returned).
>
> What is the proper way of handling this?  Just keeping the code as
> is and removing the unused status variable?  Breaking the loop once
> RpcBindingFree returns something different from RPC_S_OK?  Continuing
> and returning the first / the last status different from RPC_S_OK?
>
> Gerald