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

Reply via email to