Re: Implement DceErrorInqText

[Dmitry Timoshkov]

"Bill Medland"  wrote:
> +RPC_STATUS RPC_ENTRY DceErrorInqTextW (unsigned long e, unsigned short *b)
> +{
> - DWORD count;
> - if (acceptable_rpc_code (e))

It would be much more natural to make FormatMessageW to decide whether a passed
error code is valid or not, i.e. if it exists in the system message table.
> - {
> -
>    count = FormatMessageW (FORMAT_MESSAGE_FROM_SYSTEM | 
> -
>            FORMAT_MESSAGE_IGNORE_INSERTS,
> -
>            NULL, e, 0, b, /* I don't know!!! */ 65535, NULL);

256 (or 1024) is a reasonable margin I believe, at least messages in Wine are much
shorter of even 256.
> - }
> - else
> -
>    count = 0;
> - if (!count)
> - {
> -
>    MultiByteToWideChar(CP_ACP, 0, cszInvalidCode, -1, b, 65535 /* ??? */);

A message about invalid code should be localizable as well, so it should be
either loaded by LoadString or even by FormatMessage for consistency IMO.
winerror.h from PlatformSDK has the following definition which matches your
error text ("The error specified is not a valid Windows RPC error code.\r\n"):

//
// MessageId: RPC_S_NOT_RPC_ERROR
//
// MessageText:
//
//  The error specified is not a valid Windows RPC error code.
//
#define RPC_S_NOT_RPC_ERROR              1823L

Implement DceErrorInqText

[Bill Medland]

On December 21, 2004 10:50 pm, Dmitry Timoshkov wrote:

Thanks for the feedback
> "Bill Medland"  wrote:
> > +RPC_STATUS RPC_ENTRY DceErrorInqTextW (unsigned long e, unsigned short
> > *b) +{
> > - DWORD count;
> > - if (acceptable_rpc_code (e))
>
> It would be much more natural to make FormatMessageW to decide whether a
> passed error code is valid or not, i.e. if it exists in the system message
> table.

My comments are probably not clear enough.  That function isn't asking if the 
error code is a valid error code (which is relevant to FormatMessage).  
Actually !acceptable_rpc_code is the list of error codes for which 
FormatMessage will return a valid string but that is not the string that 
DceErrorInqText returns; it returns the "not a valid rpc code" message.
I'll reword the comments and rename the function.

The model has been tested for error codes 0 to 0x10000000 against rpcrt4 
5.0.2195.6904.

I considered two models:
a.
If (valid_rpc_code)
    if (FormatMessage)
        return the message
    else
        return error
else
    return the invalid message
b.
if (FormatMessage)
    if (valid rpc code)
        return the message
    else
        return the invalid one
else
     return the invalid one.

The second model requires a much smaller list of deviations; messages for 
which FormatMessage returns a value but that value is not returned by 
DceErrorInqText.
> > - {
> > -
> >    count = FormatMessageW (FORMAT_MESSAGE_FROM_SYSTEM |
> > -
> >            FORMAT_MESSAGE_IGNORE_INSERTS,
> > -
> >            NULL, e, 0, b, /* I don't know!!! */ 65535, NULL);
>
> 256 (or 1024) is a reasonable margin I believe, at least messages in Wine
> are much shorter of even 256.

And there was me considering ULONG_MAX!!!.  Basically I am not putting in a 
limit; it isn't my place to say.  If the documentation gave a limit I'd use 
it, but since it doesn't I assume that the caller provided a sufficient 
buffer.
> > - }
> > - else
> > -
> >    count = 0;
> > - if (!count)
> > - {
> > -
> >    MultiByteToWideChar(CP_ACP, 0, cszInvalidCode, -1, b, 65535 /*
> >
> > ??? */);
>
> A message about invalid code should be localizable as well, so it should be
> either loaded by LoadString or even by FormatMessage for consistency IMO.
> winerror.h from PlatformSDK has the following definition which matches your
> error text ("The error specified is not a valid Windows RPC error
> code.\r\n"):
>
> //
> // MessageId: RPC_S_NOT_RPC_ERROR
> //
> // MessageText:
> //
> //  The error specified is not a valid Windows RPC error code.
> //
> #define RPC_S_NOT_RPC_ERROR              1823L

Aha; there it is.  Thanks.  I'll fix it up.
(And that shortens the list of deviations by 1 too)

Implement DceErrorInqText

[Alexandre Julliard]

Bill Medland  writes:
> My comments are probably not clear enough.  That function isn't asking if the 
> error code is a valid error code (which is relevant to FormatMessage).  
> Actually !acceptable_rpc_code is the list of error codes for which 
> FormatMessage will return a valid string but that is not the string that 
> DceErrorInqText returns; it returns the "not a valid rpc code" message.
> I'll reword the comments and rename the function.

I can't imagine a case where an app would depend on getting a generic
error message instead of a detailed one; it seems to me that if
FormatMessage returns something meaningful we should use it.

Implement DceErrorInqText

[Bill Medland]

On December 22, 2004 06:47 am, Alexandre Julliard wrote:
> Bill Medland  writes:
> > My comments are probably not clear enough.  That function isn't asking if
> > the error code is a valid error code (which is relevant to
> > FormatMessage). Actually !acceptable_rpc_code is the list of error codes
> > for which FormatMessage will return a valid string but that is not the
> > string that DceErrorInqText returns; it returns the "not a valid rpc
> > code" message. I'll reword the comments and rename the function.
>
> I can't imagine a case where an app would depend on getting a generic
> error message instead of a detailed one; it seems to me that if
> FormatMessage returns something meaningful we should use it.

Oh.  I thought we wanted to do the same as Windows does!!!

Don't ask me why but for some error codes (as detailed in the patch), even 
though FormatMessage can return a perfectly reasonable string, 
DceErrorInqText doesn't return it.  E.g. 7050, 7052 and 7053 but not 7051.

Implement DceErrorInqText

[Alexandre Julliard]

Bill Medland  writes:
> Oh.  I thought we wanted to do the same as Windows does!!!
>
> Don't ask me why but for some error codes (as detailed in the patch), even 
> though FormatMessage can return a perfectly reasonable string, 
> DceErrorInqText doesn't return it.  E.g. 7050, 7052 and 7053 but not 7051.

I really doubt that Windows has a list of magic codes where it
deliberately ignores the correct error from FormatMessage to return
the generic error instead. So either our FormatMessage is wrong, or
there's something else going on here that we don't understand.

Implement DceErrorInqText

[Bill Medland]

On December 22, 2004 08:25 am, Alexandre Julliard wrote:
> Bill Medland  writes:
> > Oh.  I thought we wanted to do the same as Windows does!!!
> >
> > Don't ask me why but for some error codes (as detailed in the patch),
> > even though FormatMessage can return a perfectly reasonable string,
> > DceErrorInqText doesn't return it.  E.g. 7050, 7052 and 7053 but not
> > 7051.
>
> I really doubt that Windows has a list of magic codes where it
> deliberately ignores the correct error from FormatMessage to return
> the generic error instead. So either our FormatMessage is wrong, or
> there's something else going on here that we don't understand.

Yes, you are right.  It suddenly came to me.  (It's the length of the string).
I'll resubmit (Take 3) in a couple of minutes.