tftp: explain "block# 0" codepath; report our decision to bail out to server

if blocksize option doesn't look good (it was a FIXME. +33 bytes code);
make code more readable.

1 files changed, 70 insertions, 73 deletions

diff --git a/networking/tftp.c b/networking/tftp.c
index 1f1dfff71..3fb76ecbb 100644
--- a/networking/tftp.c
+++ b/networking/tftp.c
@@ -37,15 +37,15 @@
 #define TFTP_OACK  6
 
 #if ENABLE_FEATURE_TFTP_GET && !ENABLE_FEATURE_TFTP_PUT
-#define USE_GETPUT(a)
+#define USE_GETPUT(...)
 #define CMD_GET(cmd) 1
 #define CMD_PUT(cmd) 0
 #elif !ENABLE_FEATURE_TFTP_GET && ENABLE_FEATURE_TFTP_PUT
-#define USE_GETPUT(a)
+#define USE_GETPUT(...)
 #define CMD_GET(cmd) 0
 #define CMD_PUT(cmd) 1
 #else
-#define USE_GETPUT(a) a
+#define USE_GETPUT(...) __VA_ARGS__
 /* masks coming from getpot32 */
 #define CMD_GET(cmd) ((cmd) & 1)
 #define CMD_PUT(cmd) ((cmd) & 2)
@@ -109,10 +109,7 @@ static char *tftp_option_get(char *buf, int len, const char *option)
 
 #endif
 
-static int tftp(
-#if ENABLE_FEATURE_TFTP_GET && ENABLE_FEATURE_TFTP_PUT
-                const int cmd,
-#endif
+static int tftp( USE_GETPUT(const int cmd,)
                 len_and_sockaddr *peer_lsa,
                 const char *remotefile, const int localfd,
                 unsigned port, int tftp_bufsize)
@@ -181,6 +178,9 @@ static int tftp(
         /* First packet is built, so skip packet generation */
         goto send_pkt;
 
+        /* Using mostly goto's - continue/break will be less clear
+         * in where we actually jump to */
+
         while (1) {
                 /* Build ACK or DATA */
                 cp = xbuf + 2;
@@ -203,67 +203,64 @@ static int tftp(
  send_pkt:
                 /* Send packet */
                 *((uint16_t*)xbuf) = htons(opcode); /* fill in opcode part */
-                timeout = TFTP_NUM_RETRIES;     /* re-initialize */
-                while (1) {
-                        send_len = cp - xbuf;
-                        /* nb: need to preserve send_len value in code below
-                         * for potential resend! */
+                send_len = cp - xbuf;
+                /* NB: send_len value is preserved in code below
+                 * for potential resend */
  send_again:
 #if ENABLE_DEBUG_TFTP
-                        fprintf(stderr, "sending %u bytes\n", send_len);
-                        for (cp = xbuf; cp < &xbuf[send_len]; cp++)
-                                fprintf(stderr, "%02x ", (unsigned char) *cp);
-                        fprintf(stderr, "\n");
+                fprintf(stderr, "sending %u bytes\n", send_len);
+                for (cp = xbuf; cp < &xbuf[send_len]; cp++)
+                        fprintf(stderr, "%02x ", (unsigned char) *cp);
+                fprintf(stderr, "\n");
 #endif
-                        xsendto(socketfd, xbuf, send_len, &peer_lsa->sa, peer_lsa->len);
-                        /* Was it final ACK? then exit */
-                        if (finished && (opcode == TFTP_ACK))
-                                goto ret;
+                xsendto(socketfd, xbuf, send_len, &peer_lsa->sa, peer_lsa->len);
+                /* Was it final ACK? then exit */
+                if (finished && (opcode == TFTP_ACK))
+                        goto ret;
 
-                        /* Receive packet */
+                timeout = TFTP_NUM_RETRIES;     /* re-initialize */
  recv_again:
-                        tv.tv_sec = TFTP_TIMEOUT;
-                        tv.tv_usec = 0;
-                        FD_ZERO(&rfds);
-                        FD_SET(socketfd, &rfds);
-                        switch (select(socketfd + 1, &rfds, NULL, NULL, &tv)) {
-                                unsigned from_port;
-                        case 1:
-                                from->len = peer_lsa->len;
-                                memset(&from->sa, 0, peer_lsa->len);
-                                len = recvfrom(socketfd, rbuf, tftp_bufsize, 0,
-                                                        &from->sa, &from->len);
-                                if (len < 0) {
-                                        bb_perror_msg("recvfrom");
-                                        goto ret;
-                                }
-                                from_port = get_nport(&from->sa);
-                                if (port == org_port) {
-                                        /* Our first query went to port 69
-                                         * but reply will come from different one.
-                                         * Remember and use this new port */
-                                        port = from_port;
-                                        set_nport(peer_lsa, from_port);
-                                }
-                                if (port != from_port)
-                                        goto recv_again;
-                                goto recvd_good;
-                        case 0:
-                                timeout--;
-                                if (timeout == 0) {
-                                        bb_error_msg("last timeout");
-                                        goto ret;
-                                }
-                                bb_error_msg("last timeout" + 5);
-                                goto send_again; /* resend last sent pkt */
-                        default:
-                                bb_perror_msg("select");
+                /* Receive packet */
+                tv.tv_sec = TFTP_TIMEOUT;
+                tv.tv_usec = 0;
+                FD_ZERO(&rfds);
+                FD_SET(socketfd, &rfds);
+                switch (select(socketfd + 1, &rfds, NULL, NULL, &tv)) {
+                        unsigned from_port;
+                case 1:
+                        from->len = peer_lsa->len;
+                        memset(&from->sa, 0, peer_lsa->len);
+                        len = recvfrom(socketfd, rbuf, tftp_bufsize, 0,
+                                                &from->sa, &from->len);
+                        if (len < 0) {
+                                bb_perror_msg("recvfrom");
                                 goto ret;
                         }
-                } /* while we don't see recv packet with correct port# */
-
+                        from_port = get_nport(&from->sa);
+                        if (port == org_port) {
+                                /* Our first query went to port 69
+                                 * but reply will come from different one.
+                                 * Remember and use this new port */
+                                port = from_port;
+                                set_nport(peer_lsa, from_port);
+                        }
+                        if (port != from_port)
+                                goto recv_again;
+                        goto process_pkt;
+                case 0:
+                        timeout--;
+                        if (timeout == 0) {
+                                bb_error_msg("last timeout");
+                                goto ret;
+                        }
+                        bb_error_msg("last timeout" + 5);
+                        goto send_again; /* resend last sent pkt */
+                default:
+                        bb_perror_msg("select");
+                        goto ret;
+                }
+ process_pkt:
                 /* Process recv'ed packet */
- recvd_good:
                 opcode = ntohs( ((uint16_t*)rbuf)[0] );
                 recv_blk = ntohs( ((uint16_t*)rbuf)[1] );
 
@@ -281,9 +278,9 @@ static int tftp(
                                 "unknown transfer id",
                                 "file already exists",
                                 "no such user",
+                                "bad option",
                         };
                         enum { NUM_ERRCODE = sizeof(errcode_str) / sizeof(errcode_str[0]) };
-
                         const char *msg = "";
 
                         if (rbuf[4] != '\0') {
@@ -308,8 +305,13 @@ static int tftp(
                                 if (res) {
                                         int blksize = xatoi_u(res);
                                         if (!tftp_blocksize_check(blksize, tftp_bufsize - 4)) {
+                                                /* send ERROR 8 to server... */
+                                                /* htons can be impossible to use in const initializer: */
+                                                /*static const uint16_t error_8[2] = { htons(TFTP_ERROR), htons(8) };*/
+                                                /* thus we open-code big-endian layout */
+                                                static const char error_8[4] = { 0,TFTP_ERROR, 0,8 };
+                                                xsendto(socketfd, error_8, 4, &peer_lsa->sa, peer_lsa->len);
                                                 bb_error_msg("server proposes bad blksize %d, exiting", blksize);
-                                                // FIXME: must also send ERROR 8 to server...
                                                 goto ret;
                                         }
 #if ENABLE_DEBUG_TFTP
@@ -317,7 +319,8 @@ static int tftp(
                                                         blksize);
 #endif
                                         tftp_bufsize = blksize + 4;
-                                        block_nr = 0; // TODO: explain why???
+                                        /* Send ACK for OACK ("block" no: 0) */
+                                        block_nr = 0;
                                         continue;
                                 }
                                 /* rfc2347:
@@ -430,9 +433,8 @@ int tftp_main(int argc, char **argv)
         if (!remotefile || !argv[0])
                 bb_show_usage();
 
-        if (LONE_DASH(localfile)) {
-                fd = CMD_GET(cmd) ? STDOUT_FILENO : STDIN_FILENO;
-        } else {
+        fd = CMD_GET(cmd) ? STDOUT_FILENO : STDIN_FILENO;
+        if (!LONE_DASH(localfile)) {
                 fd = xopen(localfile, flags);
         }
 
@@ -440,17 +442,12 @@ int tftp_main(int argc, char **argv)
         peer_lsa = xhost2sockaddr(argv[0], port);
 
 #if ENABLE_DEBUG_TFTP
-        fprintf(stderr, "using server \"%s\", "
-                        "remotefile \"%s\", localfile \"%s\"\n",
+        fprintf(stderr, "using server '%s', remotefile '%s', localfile '%s'\n",
                         xmalloc_sockaddr2dotted(&peer_lsa->sa, peer_lsa->len),
                         remotefile, localfile);
 #endif
 
-        result = tftp(
-#if ENABLE_FEATURE_TFTP_GET && ENABLE_FEATURE_TFTP_PUT
-                cmd,
-#endif
-                peer_lsa, remotefile, fd, port, blocksize);
+        result = tftp( USE_GETPUT(cmd,) peer_lsa, remotefile, fd, port, blocksize);
 
         if (ENABLE_FEATURE_CLEAN_UP)
                 close(fd);