Re: cvs commit: squid3/src ftp.cc

From: Henrik Nordstrom <henrik@dont-contact.us>
Date: Tue, 14 Aug 2007 17:58:15 +0200

On tis, 2007-08-14 at 23:01 +1200, Amos Jeffries wrote:
> Alex Rousskov wrote:
> > On Mon, 2007-08-13 at 12:01 +0200, Guido Serassio wrote:
> >
> >> Now fixed.
> >
> > If you have time, it may be better to replace a virtual
> > FtpStateData::haveControlChannel(char*) into a static
> > FtpStateData::HaveControlChannel(FtpStateData*, char*) and let it check
> > the ftpState pointer itself. This will avoid more code repetition and
> > may even work faster.
>
> Whats with the uppercasing? the method is not a type. Is this a new
> convention not mentioned in the wiki/devel-faq?

Hmm.. it should be in the style guide somewhere.

object local methods and variables start with a lowercase, global
variables and static class members with an upper case.

> The FTP code is all a it messy still with its partial conversion and
> overlaps with other protocols code.

Can you be specific on what you think need further conversion or
overlaps?

> Ah, quite right. doneWithServer() is only one of the cases that method
> should handle.
> I'll be adding the other soon.

And quite likely the whole function is not needed any more after the
state fixes, but it's a reasonable safeguard so I don't object it's
addition.

Regards
Henrik

Received on Tue Aug 14 2007 - 09:58:26 MDT

This archive was generated by hypermail pre-2.1.9 : Fri Aug 31 2007 - 12:00:05 MDT