From 286cc44f5499299468e40de79b7638ec6e61d245 Mon Sep 17 00:00:00 2001
From: sudden6 <sudden6@gmx.at>
Date: Wed, 30 Dec 2020 02:42:39 +0100
Subject: [PATCH] rewrite filetransfer logic in Messenger.c

Makes the logic easier to follow and removes suspicious case.
---
 toxcore/Messenger.c | 50 +++++++++++++++++----------------------------
 toxcore/Messenger.h |  1 -
 2 files changed, 19 insertions(+), 32 deletions(-)

diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c
index 0237db7a..3837acc2 100644
--- a/toxcore/Messenger.c
+++ b/toxcore/Messenger.c
@@ -1153,8 +1153,6 @@ long int new_filesender(const Messenger *m, int32_t friendnumber, uint32_t file_
 
     ft->requested = 0;
 
-    ft->slots_allocated = 0;
-
     ft->paused = FILE_PAUSE_NOT;
 
     memcpy(ft->id, file_id, FILE_ID_LENGTH);
@@ -1435,10 +1433,6 @@ int file_data(const Messenger *m, int32_t friendnumber, uint32_t filenumber, uin
         // TODO(irungentoo): record packet ids to check if other received complete file.
         ft->transferred += length;
 
-        if (ft->slots_allocated) {
-            --ft->slots_allocated;
-        }
-
         if (length != MAX_FILE_DATA_SIZE || ft->size == ft->transferred) {
             ft->status = FILESTATUS_FINISHED;
             ft->last_packet_number = ret;
@@ -1461,9 +1455,8 @@ int file_data(const Messenger *m, int32_t friendnumber, uint32_t filenumber, uin
  * @param userdata The client userdata to pass along to chunk request callbacks.
  * @param free_slots A pointer to the number of free send queue slots in the
  *   crypto connection.
+ * @return true if there's still work to do, false otherwise.
  *
- * @return true if there are still file transfers ongoing, false if all file
- *   transfers are complete.
  */
 static bool do_all_filetransfers(Messenger *m, int32_t friendnumber, void *userdata, uint32_t *free_slots)
 {
@@ -1472,7 +1465,18 @@ static bool do_all_filetransfers(Messenger *m, int32_t friendnumber, void *userd
     // Iterate over file transfers as long as we're sending files
     for (uint32_t i = 0; i < MAX_CONCURRENT_FILE_PIPES; ++i) {
         if (friendcon->num_sending_files == 0) {
-            // no active file transfers
+            // no active file transfers anymore
+            return false;
+        }
+
+        if (*free_slots == 0) {
+            // send buffer full enough
+            return false;
+        }
+
+        if (max_speed_reached(m->net_crypto, friend_connection_crypt_connection_id(
+                                  m->fr_c, friendcon->friendcon_id))) {
+            // connection doesn't support any more data
             return false;
         }
 
@@ -1489,22 +1493,7 @@ static bool do_all_filetransfers(Messenger *m, int32_t friendnumber, void *userd
             --friendcon->num_sending_files;
         }
 
-        // Any status other than NONE means the file transfer is active.
-        if (ft->status != FILESTATUS_NONE) {
-            // Decrease free slots by the number of slots this FT uses.
-            *free_slots = max_s32(0, (int32_t) * free_slots - ft->slots_allocated);
-        }
-
         if (ft->status == FILESTATUS_TRANSFERRING && ft->paused == FILE_PAUSE_NOT) {
-            if (max_speed_reached(m->net_crypto, friend_connection_crypt_connection_id(
-                                      m->fr_c, friendcon->friendcon_id))) {
-                *free_slots = 0;
-            }
-
-            if (*free_slots == 0) {
-                continue;
-            }
-
             if (ft->size == 0) {
                 /* Send 0 data to friend if file is 0 length. */
                 file_data(m, friendnumber, i, 0, nullptr, 0);
@@ -1516,9 +1505,6 @@ static bool do_all_filetransfers(Messenger *m, int32_t friendnumber, void *userd
                 continue;
             }
 
-            // Allocate 1 slot to this file transfer.
-            ++ft->slots_allocated;
-
             const uint16_t length = min_u64(ft->size - ft->requested, MAX_FILE_DATA_SIZE);
             const uint64_t position = ft->requested;
             ft->requested += length;
@@ -1532,7 +1518,7 @@ static bool do_all_filetransfers(Messenger *m, int32_t friendnumber, void *userd
         }
     }
 
-    return friendcon->num_sending_files > 0;
+    return true;
 }
 
 static void do_reqchunk_filecb(Messenger *m, int32_t friendnumber, void *userdata)
@@ -1561,10 +1547,12 @@ static void do_reqchunk_filecb(Messenger *m, int32_t friendnumber, void *userdat
     // Request up to that number of chunks per file from the client
     const uint32_t max_ft_loops = 16;
 
-    for(uint32_t i = 0; i < max_ft_loops; ++i) {
-        do_all_filetransfers(m, friendnumber, userdata, &free_slots);
+    for (uint32_t i = 0; i < max_ft_loops; ++i) {
+        if (!do_all_filetransfers(m, friendnumber, userdata, &free_slots)) {
+            break;
+        }
 
-        if(free_slots == 0) {
+        if (free_slots == 0) {
             // stop when the buffer is full enough
             break;
         }
diff --git a/toxcore/Messenger.h b/toxcore/Messenger.h
index b9b2f23f..673fd425 100644
--- a/toxcore/Messenger.h
+++ b/toxcore/Messenger.h
@@ -131,7 +131,6 @@ struct File_Transfers {
     uint8_t paused; /* 0: not paused, 1 = paused by us, 2 = paused by other, 3 = paused by both. */
     uint32_t last_packet_number; /* number of the last packet sent. */
     uint64_t requested; /* total data requested by the request chunk callback */
-    unsigned int slots_allocated; /* number of slots allocated to this transfer. */
     uint8_t id[FILE_ID_LENGTH];
 };
 typedef enum Filestatus {