void dispatch_rpc() returns a value

[Bang Jun-Young]

Hi,

I wonder how this has been possible so far:

static void __RPC_STUB dispatch_rpc(RPC_MESSAGE *msg)
{
    struct dispatch_params *params;
    APARTMENT *apt;
    IPID ipid;
    HRESULT hr;

RpcBindingInqObject(msg->Handle, &ipid);

TRACE("ipid = %s, iMethod = %d\n", debugstr_guid(&ipid), msg->ProcNum);

params = HeapAlloc(GetProcessHeap(), 0, sizeof(*params));
    if (!params) return RpcRaiseException(E_OUTOFMEMORY);

hr = ipid_get_dispatch_params(&ipid, &apt, &params->stub, &params->chan);
    if (hr != S_OK)
    {
        ERR("no apartment found for ipid %s\n", debugstr_guid(&ipid));
        return RpcRaiseException(hr);
    }

dispatch_rpc() shouldn't return anything since it's a void function,
but it actually does.

Jun-Young

[Robert Shearman]

Bang Jun-Young wrote:
> Hi,
>
> I wonder how this has been possible so far:
>
> static void __RPC_STUB dispatch_rpc(RPC_MESSAGE *msg)
> {
>    struct dispatch_params *params;
>    APARTMENT *apt;
>    IPID ipid;
>    HRESULT hr;
>
> RpcBindingInqObject(msg->Handle, &ipid);
>
> TRACE("ipid = %s, iMethod = %d\n", debugstr_guid(&ipid), 
> msg->ProcNum);
>
> params = HeapAlloc(GetProcessHeap(), 0, sizeof(*params));
>    if (!params) return RpcRaiseException(E_OUTOFMEMORY);
>
> hr = ipid_get_dispatch_params(&ipid, &apt, &params->stub, 
> &params->chan);
>    if (hr != S_OK)
>    {
>        ERR("no apartment found for ipid %s\n", debugstr_guid(&ipid));
>        return RpcRaiseException(hr);
>    }
>
> dispatch_rpc() shouldn't return anything since it's a void function,
> but it actually does.

Thank you for reviewing the code.

RpcRaiseException is also a void function.

Rather than write:
RpcRaiseException(hr);
return;

I chose to write:
return RpcRaiseException(hr);

I didn't think the latter would be confusing as I assumed everyone would 
know that C compilers would warn if you tried to return a vale from a 
void function. Also, even if it did return something when it shouldn't, 
it wouldn't affect the calling function on x86 since the eax register 
would have been assumed to be trashed anyway.

[Bill Medland]

On May 27, 2006 03:51 pm, Robert Shearman wrote:
> Bang Jun-Young wrote:
> > Hi,
> >
> > I wonder how this has been possible so far:

snip
> > if (!params) return RpcRaiseException(E_OUTOFMEMORY);

snip
> > debugstr_guid(&ipid)); return RpcRaiseException(hr);

snip
> > dispatch_rpc() shouldn't return anything since it's a void
> > function, but it actually does.
>
> Thank you for reviewing the code.
>
> RpcRaiseException is also a void function.
>
> Rather than write:
> RpcRaiseException(hr);
> return;
>
> I chose to write:
> return RpcRaiseException(hr);
>
> I didn't think the latter would be confusing as I assumed
> everyone would know that C compilers would warn if you tried
> to return a vale from a void function. Also, even if it did
> return something when it shouldn't, it wouldn't affect the
> calling function on x86 since the eax register would have been
> assumed to be trashed anyway.

I find it confusing and the C standard (1999) explicitly states 
that it is not permitted.

[Robert Shearman]

Bill Medland wrote:
> On May 27, 2006 03:51 pm, Robert Shearman wrote:
> > Bang Jun-Young wrote:
> > > dispatch_rpc() shouldn't return anything since it's a void
> > > function, but it actually does.
> >
> > Thank you for reviewing the code.
> >
> > RpcRaiseException is also a void function.
> >
> > Rather than write:
> > RpcRaiseException(hr);
> > return;
> >
> > I chose to write:
> > return RpcRaiseException(hr);
> >
> > I didn't think the latter would be confusing as I assumed
> > everyone would know that C compilers would warn if you tried
> > to return a vale from a void function. Also, even if it did
> > return something when it shouldn't, it wouldn't affect the
> > calling function on x86 since the eax register would have been
> > assumed to be trashed anyway.
>
> I find it confusing and the C standard (1999) explicitly states 
> that it is not permitted.

Other people differ:

http://www.ussg.iu.edu/hypermail/linux/kernel/0406.1/1637.html