Re: denyMessage() vs setDenyMessage()

On 21/11/2012 6:17 p.m., Alex Rousskov wrote:
> 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?

AFAIK they are optional calls to set the message text on error pages
which almost nobody ever sees - except in the debugging process when
they actually need the text :-(

>
>> 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.

Yeah. sans bugs of course as always.

Amos
Received on Wed Nov 21 2012 - 05:38:43 MST