Removing redundancy in FTP upload code

Source commit: 212ac959f61d892841f843c0c4cc821b888fe080

source/filezilla/FtpControlSocket.cpp

@@ -5705,7 +5705,7 @@ _int64 CFtpControlSocket::GetSpeedLimit(enum transferDirection direction, CTime
   return ( _int64)1000000000000;
 }
 
-_int64 CFtpControlSocket::GetAbleToUDSize( bool &beenWaiting, CTime &curTime, _int64 &curLimit, std::list<CFtpControlSocket::t_ActiveList>::iterator &iter, enum transferDirection direction, int nBufSize)
+_int64 CFtpControlSocket::GetAbleToUDSize( bool &beenWaiting, CTime &curTime, _int64 &curLimit, std::list<CFtpControlSocket::t_ActiveList>::iterator &iter, enum transferDirection direction)
 {
   beenWaiting = false;
 
@@ -5790,15 +5790,12 @@ _int64 CFtpControlSocket::GetAbleToUDSize( bool &beenWaiting, CTime &curTime, _i
 
   curTime = nowTime;
 
-  if (!nBufSize)
-    nBufSize = BUFSIZE;
-  if (ableToRead > nBufSize)
-    ableToRead = nBufSize;
+  ableToRead = std::min(ableToRead, static_cast<_int64>(BUFSIZE));
 
   return ableToRead;
 }
 
-_int64 CFtpControlSocket::GetAbleToTransferSize(enum transferDirection direction, bool &beenWaiting, int nBufSize)
+_int64 CFtpControlSocket::GetAbleToTransferSize(enum transferDirection direction, bool &beenWaiting)
 {
   m_SpeedLimitSync.Lock();
   std::list<t_ActiveList>::iterator iter;
@@ -5816,7 +5813,7 @@ _int64 CFtpControlSocket::GetAbleToTransferSize(enum transferDirection direction
     iter = m_InstanceList[direction].end();
     iter--;
   }
-  _int64 limit = GetAbleToUDSize(beenWaiting, m_CurrentTransferTime[direction], m_CurrentTransferLimit[direction], iter, direction, nBufSize);
+  _int64 limit = GetAbleToUDSize(beenWaiting, m_CurrentTransferTime[direction], m_CurrentTransferLimit[direction], iter, direction);
   m_SpeedLimitSync.Unlock();
   return limit;
 }

source/filezilla/FtpControlSocket.h

@@ -95,7 +95,7 @@ public:
 
   _int64 GetSpeedLimit(enum transferDirection direction, CTime & time);
 
-  _int64 GetAbleToTransferSize(enum transferDirection direction, bool &beenWaiting, int nBufSize = 0);
+  _int64 GetAbleToTransferSize(enum transferDirection direction, bool &beenWaiting);
 
   t_server GetCurrentServer();
   CFtpListResult * CreateListResult(bool mlst);
@@ -168,7 +168,7 @@ protected:
   static CTime m_CurrentTransferTime[2];
   static _int64 m_CurrentTransferLimit[2];
   static CCriticalSectionWrapper m_SpeedLimitSync;
-  _int64 GetAbleToUDSize(bool & beenWaiting, CTime & curTime, _int64 & curLimit, std::list<t_ActiveList>::iterator & iter, enum transferDirection direction, int nBufSize);
+  _int64 GetAbleToUDSize(bool & beenWaiting, CTime & curTime, _int64 & curLimit, std::list<t_ActiveList>::iterator & iter, enum transferDirection direction);
   _int64 GetSpeedLimit(CTime & time, int valType, int valValue);
 
   void SetDirectoryListing(t_directory * pDirectory, bool bSetWorkingDir = true);

source/filezilla/TransferSocket.cpp

@@ -73,6 +73,14 @@ CTransferSocket::~CTransferSocket()
   delete m_pListResult;
 }
 
+_int64 CTransferSocket::GetTransferSize(CFtpControlSocket::transferDirection direction, bool & beenWaiting)
+{
+  if (GetState() != closed)
+    return m_pOwner->GetAbleToTransferSize(direction, beenWaiting);
+  else
+    return BUFSIZE;
+}
+
 /////////////////////////////////////////////////////////////////////////////
 // Member-Funktion CTransferSocket
 void CTransferSocket::OnReceive(int nErrorCode)
@@ -138,11 +146,7 @@ void CTransferSocket::OnReceive(int nErrorCode)
       OnConnect(0);
 
     bool beenWaiting = false;
-    _int64 ableToRead;
-    if (GetState() != closed)
-      ableToRead = m_pOwner->GetAbleToTransferSize(CFtpControlSocket::download, beenWaiting);
-    else
-      ableToRead = BUFSIZE;
+    _int64 ableToRead = GetTransferSize(CFtpControlSocket::download, beenWaiting);
 
     if (!beenWaiting)
       DebugAssert(ableToRead);
@@ -485,51 +489,59 @@ void CTransferSocket::OnSend(int nErrorCode)
   if (!m_pBuffer)
     m_pBuffer = new char[BUFSIZE];
 
-  int numread;
+  bool firstPass = true;
 
-  bool beenWaiting = false;
-  _int64 currentBufferSize;
-  if (GetState() != closed)
-    currentBufferSize = m_pOwner->GetAbleToTransferSize(CFtpControlSocket::upload, beenWaiting);
-  else
-    currentBufferSize = BUFSIZE;
-
-  if (!currentBufferSize && !m_bufferpos)
-  {
-    // Not allowed to send yet, try later
-    TriggerEvent(FD_WRITE);
-    return;
-  }
-  else if (m_bufferpos < currentBufferSize)
+  while (TRUE)
   {
-    numread = ReadDataFromFile(m_pBuffer + m_bufferpos, static_cast<int>(currentBufferSize - m_bufferpos));
-    if (numread < 0 )
+    bool beenWaiting = false;
+    _int64 currentBufferSize = GetTransferSize(CFtpControlSocket::upload, beenWaiting);
+
+    int numread;
+    if ((currentBufferSize == 0) && (m_bufferpos == 0))
     {
+      // Not allowed to send yet, try later
+      TriggerEvent(FD_WRITE);
       return;
     }
-    else if (!numread && !m_bufferpos)
+
+    int readwanted = static_cast<int>(currentBufferSize - m_bufferpos);
+    if (m_bufferpos < currentBufferSize)
+    {
+      numread = ReadDataFromFile(m_pBuffer + m_bufferpos, readwanted);
+      if (numread < 0 )
+      {
+        return;
+      }
+      else if ((numread == 0) && (m_bufferpos == 0))
+      {
+        CloseOnShutDownOrError(0);
+        return;
+      }
+    }
+    else
     {
-      CloseOnShutDownOrError(0);
-      return;
+      numread = 0;
     }
-  }
-  else
-    numread = 0;
 
-  DebugAssert((numread+m_bufferpos) <= BUFSIZE);
-  DebugAssert(numread>=0);
-  DebugAssert(m_bufferpos>=0);
+    DebugAssert(numread>=0);
+    DebugAssert(m_bufferpos>=0);
 
-  if (numread+m_bufferpos <= 0)
-  {
-    CloseOnShutDownOrError(0);
-    return;
-  }
+    int bufferlen = (numread + m_bufferpos);
+    // Just to make the change noop, but the test should probably be done everytime
+    if (firstPass)
+    {
+      DebugAssert(bufferlen <= BUFSIZE);
+
+      if (bufferlen <= 0)
+      {
+        CloseOnShutDownOrError(0);
+        return;
+      }
+      firstPass = false;
+    }
 
-  int numsent = Send(m_pBuffer, numread + m_bufferpos);
+    int numsent = Send(m_pBuffer, bufferlen);
 
-  while (TRUE)
-  {
     if (numsent != SOCKET_ERROR)
     {
       m_pOwner->SpeedLimitAddTransferredBytes(CFtpControlSocket::upload, numsent);
@@ -567,7 +579,7 @@ void CTransferSocket::OnSend(int nErrorCode)
     }
     else
     {
-      int pos = numread + m_bufferpos - numsent;
+      int pos = bufferlen - numsent;
 
       if (pos < 0 || (numsent + pos) > BUFSIZE)
       {
@@ -576,7 +588,7 @@ void CTransferSocket::OnSend(int nErrorCode)
         return;
       }
       else if (!pos && // all data in buffer were sent
-               numread < (currentBufferSize-m_bufferpos) && // was read less then wanted (eof reached?)
+               numread < readwanted && // was read less then wanted (eof reached?)
                m_bufferpos != currentBufferSize) // and it's not because the buffer is full?
       {
         // With TLS 1.3 we can get back
@@ -605,40 +617,6 @@ void CTransferSocket::OnSend(int nErrorCode)
       return;
     }
     UpdateStatusBar(false);
-
-    if (GetState() != closed)
-      currentBufferSize = m_pOwner->GetAbleToTransferSize(CFtpControlSocket::upload, beenWaiting);
-    else
-      currentBufferSize = BUFSIZE;
-
-    if (m_bufferpos < currentBufferSize)
-    {
-      numread = ReadDataFromFile(m_pBuffer + m_bufferpos, static_cast<int>(currentBufferSize - m_bufferpos));
-      if (numread < 0 )
-      {
-        return;
-      }
-      else if (!numread && !m_bufferpos)
-      {
-        CloseOnShutDownOrError(0);
-        return;
-      }
-    }
-    else
-    {
-      numread = 0;
-    }
-
-    if (!currentBufferSize && !m_bufferpos)
-    {
-      // Not allowed to send yet, try later
-      TriggerEvent(FD_WRITE);
-      return;
-    }
-
-    DebugAssert(numread>=0);
-    DebugAssert(m_bufferpos>=0);
-    numsent = Send(m_pBuffer, numread+m_bufferpos);
   }
 
 }
@@ -898,6 +876,7 @@ int CTransferSocket::ReadData(char * buffer, int len)
   return result;
 }
 
+// Used only once now
 int CTransferSocket::ReadDataFromFile(char *buffer, int len)
 {
   try

source/filezilla/TransferSocket.h

@@ -79,6 +79,7 @@ protected:
   void EnsureSendClose(int Mode);
   void CloseOnShutDownOrError(int Mode);
   void SetBuffers();
+  _int64 GetTransferSize(CFtpControlSocket::transferDirection direction, bool & beenWaiting);
 
   LARGE_INTEGER m_LastUpdateTime;
   unsigned int m_LastSendBufferUpdate;