FTP Transaction code for new Portable FTP code....

FTP Transaction code for new Portable FTP code.

The patch is contributed by Ibrar Ahmed <ibrar.ahmad@gmail.com>;.

Original review: http://codereview.chromium.org/39130

R=wtc
http://crbug.com/4965
TEST=None.  Work in progress.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=15760

[wtc, 2009-05-08 18:23:39]

Ibrar, I'm preparing to check in your FTP CL 3.

[wtc, 2009-05-08 20:39:07]

Ibrar,

I made some changes to fix style issues and compilation errors.

Please study the diffs between Patch Set 1 (your CL) and Patch Set 3 to learn
from my changes.  I annotated some of the changes with links to our style guide.

The issues that I didn't fix are marked with TODO(ibrar) comments.  Please fix
them after I check this in.

I'm waiting for test results from the try server...

Thanks!

http://codereview.chromium.org/115137/diff/1/8
File net/ftp/ftp_network_transaction.cc (right):

http://codereview.chromium.org/115137/diff/1/8#newcode366
Line 366: } else {
No else after a return statement.

http://codereview.chromium.org/115137/diff/1033/20
File net/ftp/ftp_network_layer.cc (right):

http://codereview.chromium.org/115137/diff/1033/20#newcode22
Line 22: FtpTransactionFactory* FtpNetworkLayer::CreateFactory() {
I moved the definition of this method because "Method definitions in the
corresponding .cc file should be the same as the declaration order, as much as
possible."

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Declaration_Order

http://codereview.chromium.org/115137/diff/1033/26
File net/ftp/ftp_network_transaction.h (right):

http://codereview.chromium.org/115137/diff/1033/26#newcode30
Line 30: virtual int Start(const FtpRequestInfo* request_info,
Our style guide has one guideline for formatting function declarations and
definitions, and another guideline for function calls:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Decla...

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Calls

Many of the changes I made are related to these two guidelines.  It's important
to master when to follow which guideline.

http://codereview.chromium.org/115137/diff/1033/26#newcode43
Line 43: enum Command {
I moved these two enum type definitions to follow the style guide's guideline on
declaration order:

 Within each section, the declarations generally should be in the following
order:

    * Typedefs and Enums
    * Constants
    * Constructors
    * Destructor
    * Methods, including static methods
    * Data Members, including static data members

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Declaration_Order

[wtc, 2009-05-08 21:37:30]

Ibrar,

The following are changes I had to make to fix compilation
errors on Linux.

http://codereview.chromium.org/115137/diff/40/46
File net/ftp/ftp_network_transaction.cc (right):

http://codereview.chromium.org/115137/diff/40/46#newcode19
Line 19: #define sscanf_s sscanf
sscanf_s doesn't exist on Mac and Linux.  We should find an
alternative.

http://codereview.chromium.org/115137/diff/40/46#newcode31
Line 31: : command_sent_(COMMAND_NONE),
GCC warns if the initializer list doesn't list the members
in the same order they're declared in the header.

http://codereview.chromium.org/115137/diff/40/46#newcode579
Line 579: if (sscanf_s(ptr, "%d,%d,%d,%d,%d,%d",
The %ld format matches 'long'.  Since i0, i1, ..., p1 are all
'int', we have to use the %d format.

http://codereview.chromium.org/115137/diff/40/47
File net/ftp/ftp_network_transaction.h (right):

http://codereview.chromium.org/115137/diff/40/47#newcode147
Line 147: scoped_refptr<IOBuffer> response_message_buf_;
IOBuffer is a reference-counted type, and therefore must
always be stored in a scoped_refptr.