Re: DC++ incompatibility

On Thu Nov 04, 2004 at 09:00:11PM +0100, Tommy Thorsen wrote:

> The background for this mail is the problem described in (at least) two 
> threads in the forum, namely:
> http://dcgui.berlios.de/forum/viewtopic.php?t=2363
> and
> http://dcgui.berlios.de/forum/viewtopic.php?t=2538
> 
> I've posted using the name SuperTommy.
> 
> The function CDownloadManager::DM_TransferCallBack() in the file 
> "cdownloadmanager.cpp" (line 3785 onwards, from "case DC_MESSAGE_KEY:") 
> contains some code where the hierarchy of if's looks like this pseudo-code:
> 
> 01: if ( Transfer->GetDstDirection() == Transfer->GetSrcDirection() )
> 02: {
> 03:     if ( Transfer->GetDstLevel() < Transfer->GetSrcLevel() )
> 04:     {
> 05:         if ( Transfer->GetSrcDirection() == edDOWNLOAD )
> 06:         {
> 07:             <change DstDirection to edUPLOAD>
> 08:         }
> 09:         else if ( Transfer->GetSrcDirection() == edUPLOAD )
> 10:         {
> 11:             <change DstDirection to edDOWNLOAD>
> 12:         }
> 13:     }
> 14:     else if ( Transfer->GetDstLevel() > Transfer->GetSrcLevel() )
> 15:     {
> 16:         if ( Transfer->GetSrcDirection() == edDOWNLOAD )
> 17:         {
> 18:             <change SrcDirection to edUPLOAD>
> 19:         }
> 20:         else if ( (Transfer->GetSrcDirection() == edUPLOAD) && 
> (Transfer->GetDstDirection() == edUPLOAD) )
> 21:         {
> 22:             <output a warning>
> 23:         }
> 24:         else
> 25:         {
> 26:             <output a warning>
> 27:         }
> 28:     }
> 29: }
> 30: else
> 31: {
> 32:     bdirec = TRUE;
> 33: }
> 
> Question 1: Why is both SrcDirection and DstDirection checked on line 20 
> when we've already established that they are the same in line 1? If 
> there is a reason, why is only SrcDirection checked on line 9. This will 
> of course not create any problems, but it makes the code look 
> asymmetrical and difficult to understand for people like me, who has no 
> previous familiarity with it.
> 
No real reason I can see - it's probably just legacy code.  It made
sense at some time but the code has been altered/moved since so it's now
redundant.

> Question 2: This is really my most important question; Why do we not try 
> to change SrcDirection in line 22?
> 
Because it doesn't make sense here.  Both parties are trying to upload a
file. but the other party has priority in this case, so they should
switch direction or retry (if they're not wanting to download).

> Question 3: Why is there an else on line 24 and not after line 12 (not 
> very important, but I'm still looking for that symmetry)?
> 
Again, just legacy code I guess - it should never be reached.

> Question 4: What does SrcDirection and DstDirection do anyway? It looks 
> to me like they need do be different, right? One side of the 
> communication should upload while the other downloads, is that it?
> 
Yes, usually they'll be different.  If everyone was active then theere
wouldn't be a problem as you'd always be uploading to incoming
connections and downloading from outgoing connections.  With passive
users you just don't know whether someone connecting to you is
uploading/downloading so we get into this mess.  The SrcLevel and
DstLevel are supposed to work around this - whoever has the higher
number gets priority.

Hope that clears things up a bit.  I'm not sure what the heart of the
problem is though - I've seen one user repeatedly connect to me and ask
to upload, though I'm not wanting to download anything from them.  Could
be a bug in DC++ I guess...

Cheers,
        Robin
--

-- 
     ___        
    ( ' }     |       Robin Hill        <robin <at> robinhill.me.uk> |
   / / )      | Little Jim says ....                            |
  // !!       |      "He fallen in de water !!"                 |