Re: denyMessage() vs setDenyMessage()

On 11/20/2012 06:09 PM, Amos Jeffries wrote:
> On 21.11.2012 07:29, Alex Rousskov wrote:
>> Hello,
>>
>> Should the following calls use setDenyMessage() instead?
>>
>>> ./auth/negotiate/UserRequest.cc:
>>> auth_user_request->denyMessage("Authentication in progress");
>>> ./auth/negotiate/UserRequest.cc:
>>> auth_user_request->denyMessage("NTLM authentication requires a
>>> persistent connection");
>>> ./auth/negotiate/UserRequest.cc:
>>> auth_user_request->denyMessage("Login successful");
>>> ./auth/negotiate/UserRequest.cc:
>>> auth_user_request->denyMessage(arg);
>>> ./auth/negotiate/UserRequest.cc:
>>> auth_user_request->denyMessage(blob);
>>> ./auth/ntlm/UserRequest.cc:
>>> auth_user_request->denyMessage("Authentication in progress");
>>> ./auth/ntlm/UserRequest.cc:
>>> auth_user_request->denyMessage("NTLM authentication requires a
>>> persistent connection");
>>> ./auth/ntlm/UserRequest.cc:
>>> auth_user_request->denyMessage("Login successful");
>>> ./auth/ntlm/UserRequest.cc: auth_user_request->denyMessage(blob);
>>> ./auth/ntlm/UserRequest.cc: auth_user_request->denyMessage(blob);
>>
>>
>
> Yes they should.

Does the current code lead to bugs or are those setDenyMessage() calls
optional? If it leads to bugs, can you tell what kind of wrong behavior
should one expect?

> I think more importantly that the three forms of this accessor should be
> combined into the more normal overloaded forms ASAP:
> void denyMessage(const char *msg);
> const char *denyMessage(void);
>
> There are only three points in the code needing that default-string
> accessor and a fast inline get()?get():default pattern would seem to be
> very appropriate for all those usage points.

Sounds good to me. The argumentless version of the denyMessage() method
should also be const.

Thank you,

Alex.
Received on Wed Nov 21 2012 - 05:17:25 MST