On 12.10.2015 18:40, Amos Jeffries wrote:
On 12/10/2015 9:21 p.m., Vitaly Lavrov wrote:
On 11.10.2015 18:56, Amos Jeffries wrote:
Attached is a cleaned version of the patch. It now meets the source
coding guidelines, and applies cleanly to trunk / Squid-4.

I took the opportunity to remove two useless if() conditions - one was
duplicate checking a value, the other for values in a pointer that was
just new()'d.


Logic looks mostly okay, but this part in the middle of
Ftp::Gateway::loginFailed() is rather odd:

-    // any other problems are general falures.
-    if (!err) {
-        ftpFail(this);
+    if (!err)
           return;
-    }

The original behaviour was to output an error and perform the ftpQuit()
sequence. Now it appears to just halt without doing anything.

Is that behaviour change intentional? I suspect the correct behaviour is
to remove that if()-condition entirely, but I'm unsure.
It is possible that  ftpFail(this) was deleted accidentally during
multiple rework code.
The original version "if(!err) { ftpFail(this); return; }" looks more
logical.


Could we get that tested please before I apply the patch to Squid-4.
All right. In the case of non-standard error code ftpFail() returns an error to 
the client ERR_FTP_FAILED/502.

Thanks
Amos


_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to