Half-way tidy

diff --git a/src/net/tcp/ftp.c b/src/net/tcp/ftp.c
index 81eba4970a923164670022a99ae9bdf16db13a25..dae1714a4a5101fe273133b0853fdb30865e087b 100644 (file)
--- a/src/net/tcp/ftp.c
+++ b/src/net/tcp/ftp.c
@@ -12,25 +12,49 @@
  *
  */
 
-const char *ftp_strings[] = {
-       [FTP_CONNECT] = "",
-       [FTP_USER] = "USER anonymous\r\n",
-       [FTP_PASS] = "PASS etherboot@etherboot.org\r\n",
-       [FTP_TYPE] = "TYPE I\r\n",
-       [FTP_PASV] = "PASV\r\n",
-       [FTP_RETR] = "RETR %s\r\n",
-       [FTP_QUIT] = "QUIT\r\n",
-       [FTP_DONE] = "",
+/** An FTP control channel string */
+struct ftp_string {
+       /** String format */
+       const char *format;
+       /** Offset to string data
+        *
+        * This is the offset within the struct ftp_request to the
+        * pointer to the string data.  Use ftp_string_data() to get a
+        * pointer to the actual data.
+        */
+       off_t data_offset;
 };
 
-static inline struct ftp_request *
-tcp_to_ftp ( struct tcp_connection *conn ) {
-       return container_of ( conn, struct ftp_request, tcp );
+#define ftp_string_offset( fieldname ) \
+       offsetof ( struct ftp_request, fieldname )
+
+/**
+ * Get data associated with an FTP control channel string
+ *
+ * @v ftp              FTP request
+ * @v data_offset      Data offset field from ftp_string structure
+ * @ret data           Pointer to data
+ */
+static inline const void * ftp_string_data ( struct ftp_request *ftp,
+                                            off_t data_offset ) {
+       return * ( ( void ** ) ( ( ( void * ) ftp ) + data_offset ) );
 }
 
+/** FTP control channel strings */
+const struct ftp_string ftp_strings[] = {
+       [FTP_CONNECT]   = { "", 0 },
+       [FTP_USER]      = { "USER anonymous\r\n", 0 },
+       [FTP_PASS]      = { "PASS etherboot@etherboot.org\r\n", 0 },
+       [FTP_TYPE]      = { "TYPE I\r\n", 0 },
+       [FTP_PASV]      = { "PASV\r\n", 0 },
+       [FTP_RETR]      = { "RETR %s\r\n", ftp_string_offset ( filename ) },
+       [FTP_QUIT]      = { "QUIT\r\n", 0 },
+       [FTP_DONE]      = { "", 0 },
+};
+
 static inline struct ftp_request *
-tcp_to_ftp_data ( struct tcp_connection *conn ) {
-       return container_of ( conn, struct ftp_request, tcp_data );
+tcp_to_ftp ( struct tcp_connection *conn ) {
+       return container_of ( conn, struct ftp_request, tcp );
 }
 
 static void ftp_complete ( struct ftp_request *ftp, int complete ) {
@@ -39,85 +63,62 @@ static void ftp_complete ( struct ftp_request *ftp, int complete ) {
        tcp_close ( &ftp->tcp );
 }
 
-
-static void ftp_aborted ( struct tcp_connection *conn ) {
-       struct ftp_request *ftp = tcp_to_ftp ( conn );
-
-       ftp_complete ( ftp, -ECONNABORTED );
-}
-
-static void ftp_timedout ( struct tcp_connection *conn ) {
-       struct ftp_request *ftp = tcp_to_ftp ( conn );
-
-       ftp_complete ( ftp, -ETIMEDOUT );
-}
-
-static void ftp_closed ( struct tcp_connection *conn ) {
-       struct ftp_request *ftp = tcp_to_ftp ( conn );
-
-       ftp_complete ( ftp, 1 );
-}
-
-static void ftp_acked ( struct tcp_connection *conn, size_t len ) {
-       struct ftp_request *ftp = tcp_to_ftp ( conn );
-       
-       ftp->already_sent += len;
-}
-
-int ftp_open_passive ( struct ftp_request *ftp ) {
-       char *ptr = ftp->passive_text;
-       uint8_t *byte = ( uint8_t * ) ( &ftp->tcp_data.sin );
-       int i;
-
-       /* Parse the IP address and port from the PASV repsonse */
-       for ( i = 6 ; i ; i-- ) {
-               if ( ! *ptr )
-                       return -EINVAL;
-               *(byte++) = strtoul ( ptr, &ptr, 10 );
-               if ( *ptr )
-                       ptr++;
-       }
-       if ( *ptr )
-               return -EINVAL;
-
-       tcp_connect ( &ftp->tcp_data );
-       return 0;
+/**
+ * Parse FTP byte sequence value
+ *
+ * @v text     Text string
+ * @v value    Value buffer
+ * @v len      Length of value buffer
+ *
+ * This parses an FTP byte sequence value (e.g. the "aaa,bbb,ccc,ddd"
+ * form for IP addresses in PORT commands) into a byte sequence.  @c
+ * *text will be updated to point beyond the end of the parsed byte
+ * sequence.
+ *
+ * This function is safe in the presence of malformed data, though the
+ * output is undefined.
+ */
+static void ftp_parse_value ( char **text, uint8_t *value, size_t len ) {
+       do {
+               *(value++) = strtoul ( *text, text, 10 );
+               if ( **text )
+                       (*text)++;
+       } while ( --len );
 }
 
-void ftp_reply ( struct ftp_request *ftp ) {
+/**
+ * Handle a response from an FTP server
+ *
+ * @v ftp      FTP request
+ *
+ * This is called once we have received a complete repsonse line.
+ */
+static void ftp_reply ( struct ftp_request *ftp ) {
        char status_major = ftp->status_text[0];
-       int success;
 
-       /* Ignore "intermediate" messages */
+       /* Ignore "intermediate" responses (1xx codes) */
        if ( status_major == '1' )
                return;
 
-       /* Check for success */
-       success = ( status_major == '2' );
-
-       /* Special-case the "USER"-"PASS" sequence */
-       if ( ftp->state == FTP_USER ) {
-               if ( success ) {
-                       /* No password was asked for; pretend we have
-                        * already entered it
-                        */
-                       ftp->state = FTP_PASS;
-               } else if ( status_major == '3' ) {
-                       /* Password requested, treat this as success
-                        * for our purposes
-                        */
-                       success = 1;
-               }
-       }
-       
-       /* Abort on failure */
-       if ( ! success )
+       /* Anything other than success (2xx) or, in the case of a
+        * repsonse to a "USER" command, a password prompt (3xx), is a
+        * fatal error.
+        */
+       if ( ! ( ( status_major == '2' ) ||
+                ( ( status_major == '3' ) && ( ftp->state == FTP_USER ) ) ) )
                goto err;
 
        /* Open passive connection when we get "PASV" response */
        if ( ftp->state == FTP_PASV ) {
-               if ( ftp_open_passive ( ftp ) != 0 )
-                       goto err;
+               char *ptr = ftp->passive_text;
+
+               ftp_parse_value ( &ptr,
+                                 ( uint8_t * ) &ftp->tcp_data.sin.sin_addr,
+                                 sizeof ( ftp->tcp_data.sin.sin_addr ) );
+               ftp_parse_value ( &ptr,
+                                 ( uint8_t * ) &ftp->tcp_data.sin.sin_port,
+                                 sizeof ( ftp->tcp_data.sin.sin_port ) );
+               tcp_connect ( &ftp->tcp_data );
        }
 
        /* Move to next state */
@@ -127,59 +128,96 @@ void ftp_reply ( struct ftp_request *ftp ) {
        return;
 
  err:
-       ftp->complete = -EPROTO;
-       tcp_close ( &ftp->tcp );
+       /* Flag protocol error and close connections */
+       ftp_complete ( ftp, -EPROTO );
 }
 
 static void ftp_newdata ( struct tcp_connection *conn,
                          void *data, size_t len ) {
        struct ftp_request *ftp = tcp_to_ftp ( conn );
+       char *recvbuf = ftp->recvbuf;
+       size_t recvsize = ftp->recvsize;
        char c;
-
-       for ( ; len ; data++, len-- ) {
-               c = * ( ( char * ) data );
-               if ( ( c == '\r' ) || ( c == '\n' ) ) {
-                       if ( ftp->recvsize == 0 )
+       
+       while ( len-- ) {
+               c = * ( ( char * ) data++ );
+               switch ( c ) {
+               case '\r' :
+               case '\n' :
+                       /* End of line: call ftp_reply() to handle
+                        * completed reply.  Avoid calling ftp_reply()
+                        * twice if we receive both \r and \n.
+                        */
+                       if ( recvsize == 0 )
                                ftp_reply ( ftp );
-                       ftp->recvbuf = ftp->status_text;
-                       ftp->recvsize = sizeof ( ftp->status_text ) - 1;
-               } else if ( c == '(' ) {
-                       ftp->recvbuf = ftp->passive_text;
-                       ftp->recvsize = sizeof ( ftp->passive_text ) - 1;
-               } else if ( c == ')' ) {
-                       ftp->recvsize = 0;
-               } else if ( ftp->recvsize > 0 ) {
-                       *(ftp->recvbuf++) = c;
-                       ftp->recvsize--;
+                       /* Start filling up the status code buffer */
+                       recvbuf = ftp->status_text;
+                       recvsize = sizeof ( ftp->status_text ) - 1;
+                       break;
+               case '(' :
+                       /* Start filling up the passive parameter buffer */
+                       recvbuf = ftp->passive_text;
+                       recvsize = sizeof ( ftp->passive_text ) - 1;
+                       break;
+               case ')' :
+                       /* Stop filling the passive parameter buffer */
+                       recvsize = 0;
+                       break;
+               default :
+                       /* Fill up buffer if applicable */
+                       if ( recvsize > 0 ) {
+                               *(recvbuf++) = c;
+                               recvsize--;
+                       }
+                       break;
                }
        }
+
+       /* Store for next invocation */
+       ftp->recvbuf = recvbuf;
+       ftp->recvsize = recvsize;
+}
+
+static void ftp_acked ( struct tcp_connection *conn, size_t len ) {
+       struct ftp_request *ftp = tcp_to_ftp ( conn );
+       
+       /* Mark off ACKed portion of the currently-transmitted data */
+       ftp->already_sent += len;
 }
 
 static void ftp_senddata ( struct tcp_connection *conn ) {
        struct ftp_request *ftp = tcp_to_ftp ( conn );
-       const char *format;
-       const char *data;
+       const struct ftp_string *string;
        size_t len;
 
-       /* Select message format string and data */
-       format = ftp_strings[ftp->state];
-       switch ( ftp->state ) {
-       case FTP_RETR:
-               data = ftp->filename;
-               break;
-       default:
-               data = NULL;
-               break;
-       }
-       if ( ! data )
-               data = "";
-       
-       /* Build message */
-       len = snprintf ( tcp_buffer, tcp_buflen, format, data );
+       /* Send the as-yet-unACKed portion of the string for the
+        * current state.
+        */
+       string = &ftp_strings[ftp->state];
+       len = snprintf ( tcp_buffer, tcp_buflen, string->format,
+                        ftp_string_data ( ftp, string->data_offset ) );
        tcp_send ( conn, tcp_buffer + ftp->already_sent,
                   len - ftp->already_sent );
 }
 
+static void ftp_aborted ( struct tcp_connection *conn ) {
+       struct ftp_request *ftp = tcp_to_ftp ( conn );
+
+       ftp_complete ( ftp, -ECONNABORTED );
+}
+
+static void ftp_timedout ( struct tcp_connection *conn ) {
+       struct ftp_request *ftp = tcp_to_ftp ( conn );
+
+       ftp_complete ( ftp, -ETIMEDOUT );
+}
+
+static void ftp_closed ( struct tcp_connection *conn ) {
+       struct ftp_request *ftp = tcp_to_ftp ( conn );
+
+       ftp_complete ( ftp, 1 );
+}
+
 static struct tcp_operations ftp_tcp_operations = {
        .aborted        = ftp_aborted,
        .timedout       = ftp_timedout,
@@ -189,6 +227,11 @@ static struct tcp_operations ftp_tcp_operations = {
        .senddata       = ftp_senddata,
 };
 
+static inline struct ftp_request *
+tcp_to_ftp_data ( struct tcp_connection *conn ) {
+       return container_of ( conn, struct ftp_request, tcp_data );
+}
+
 static void ftp_data_aborted ( struct tcp_connection *conn ) {
        struct ftp_request *ftp = tcp_to_ftp_data ( conn );