Current Source Code
#401
Posted 27 April 2015 - 04:14 AM
#402
Posted 28 April 2015 - 04:43 PM
Some Support, on 25 April 2015 - 06:11 PM, said:
It is not a trivial in general to test in the wild on 100 Mbit/s for like 40+ downloaders condition since hardly seen that many at the same time and 9900 KB/s never reached yet so no "clients waiting in queue" condition met. Some results as per below could be interesting however (dev 16; Connection-Capacities-Upload->9900 KB/s).
Check 1.
Upload limit is 9900 KB/s (no USS) and 4 downloaders - A (low id); B (low id); C; D (low id) -> 1-1. A ~ 40 KB/s; B ~ 2,29 MB/s; C ~ 317 KB/s; D ~ 20 KB/s -> then upload limit removed -> 1-2. A ~ 19 KB/s; B ~ 141 KB/s; C ~ 141 KB/s; D ~ 9 KB/s -> then upload limit set to 9900 KB/s -> 1-3. A ~ 48 KB/s; B ~ 2,39 MB/s; C ~ 1,08 MB/s; D ~ 20 KB/s Repeated after some time with upload limit remained 9900 KB/s - C left already; E (low id) entered -> 1-4. A ~ 48 KB/s; B ~ 2,43 MB/s; E ~ 13 KB/s; D ~ 21 KB/s -> then upload limit removed -> 1-5. A ~ 20 KB/s; B ~ 285 KB/s; E ~ 8 KB/s; D ~ 8 KB/s -> then upload limit set to 9900 KB/s -> 1-6. A ~ 47 KB/s; B ~ 2,56 MB/s; E ~ 21 KB/s; D ~ 14 KB/s
The above results were quite confusing due to huge upload drop for removed limit (unlimited upload), but above was with UploadCapacity=25 leftover from previous eMule versions as discovered later comparing preferences.ini with fresh from 0.50b BETA1 install. Removed UploadCapacity=25 so only UploadCapacityNew=9900 remained and performed the further checks (Sidenote: OverlappedSockets=1 dev 16 crashed with removed UploadCapacity=25 leftover anyway).
Check 2.
Upload limit is 9900 KB/s (no USS) and 2 downloaders - A (low id); B (low id) -> 2-1. A ~ 50 KB/s; B ~ 5,7 MB/s -> then upload limit removed -> 2-2. A ~ 50 KB/s; B ~ 5,7 MB/s -> then enabled USS (lowest allowed upload speed = 1) -> 2-3. A ~ 50 KB/s; B ~ 5 MB/s -> then changed USS lowest allowed upload speed to 9900 -> 2-4. A ~ 50 KB/s; B ~ 5,7 MB/s -> then upload limit set to 1024 KB/s; disabled USS -> 2-5. A ~ 50 KB/s; B ~ 970 KB/s
Check 3.
Upload limit is 9900 KB/s (no USS) and 3 downloaders - A; B (low id); C -> 3-1. A ~ 60 KB/s; B ~ 60 KB/s; C ~ 3,6 MB/s -> then D (low id) entered -> 3-2. A ~ 60 KB/s; B ~ 60 KB/s; C ~ 2,6 MB/s; D ~ 4,0 MB/s -> then upload limit removed -> 3-3. A ~ 60 KB/s; B ~ 60 KB/s; C ~ 2,7 MB/s; D ~ 3,5 MB/s -> then enabled USS (lowest allowed upload speed = 1) -> 3-4. A ~ 60 KB/s; B ~ 60 KB/s; C ~ 2,4 MB/s; D ~ 2,5 MB/s -> then changed USS lowest allowed upload speed to 9900 -> 3-5. A ~ 60 KB/s; B ~ 60 KB/s; C ~ 2,6 MB/s; D ~ 3,5 MB/s -> then upload limit set to 9900 KB/s -> 3-6. A ~ 60 KB/s; B ~ 60 KB/s; C ~ 2,6 MB/s; D ~ 4,0 MB/s -> then upload limit set to 1024 KB/s; disabled USS -> 3-7. A ~ 15 KB/s; B ~ 30 KB/s; C ~ 660 KB/s; D ~ 320 KB/s -> then upload limit set to 9900 KB/s -> 3-8. A ~ 30 KB/s; B ~ 60 KB/s; C ~ 2,6 MB/s; D ~ 4,0 MB/s -> then D (low id) left -> 3-9. A ~ 30 KB/s; B ~ 60 KB/s; C ~ 3,6 MB/s -> then B (low id) left; E (low id) entered -> 3-10. A ~ 30 KB/s; E ~ 430 KB/s; C ~ 3,6 MB/s -> then F (low id) entered -> 3-11. A ~ 30 KB/s; E ~ 430 KB/s; C ~ 2,6 MB/s; F ~ 4,0 MB/s
Never performed such tests prior dev 16 so no info for slots vs speed to compare. Result 3-11 looks promising as higher speed downloaders A, B (C seems capped already as 3-8 vs 3-9; probably D and F were capped aswell) or more opened slots could lead to better upload capacity accommodation.
Hope it helps.
This post has been edited by Ergol: 28 April 2015 - 06:39 PM
#403
Posted 28 April 2015 - 06:59 PM
Ergol, on 28 April 2015 - 05:43 PM, said:
Thanks for your work. The general upload speed and capacity should not have changed in build 16, but only the average slot speed. But of course that comes only into play when all slots are taken, otherwise the additional bandwidth is distributed to the active slots. 3-4 open slots isn't quite enough for such a test, there should be a waiting queue. But I understand that depending on which files and how many files you share this is hard to test.
#404
Posted 29 April 2015 - 07:02 PM
try to put in your incomming folder some linux-s distro
#405
Posted 01 May 2015 - 06:43 AM
#406
Posted 02 May 2015 - 04:54 PM
1. File DownloadClient.cpp
Method void CUpDownClient::DrawStatusBar(CDC* dc, LPCRECT rect, bool onlygreyrect, bool bFlat) const
ASSERT(reqfile); s_StatusBar.SetFileSize(reqfile->GetFileSize()); s_StatusBar.SetHeight(rect->bottom - rect->top); s_StatusBar.SetWidth(rect->right - rect->left); s_StatusBar.Fill(crNeither); if (!onlygreyrect && reqfile && m_abyPartStatus) {
It would be safer and more logical to check reqfile for null instead or immediately after the assert to be sure about reqfile->GetFileSize().
Method void CUpDownClient::SendBlockRequests()
if (thePrefs.GetDebugClientTCPLevel() > 0) DebugSend("OP__RequestParts", this, reqfile!=NULL ? reqfile->GetFileHash() : NULL);
In this case condition reqfile!=NULL is superfluos, because it was already verified at the very beginning of the method.
2. File EmuleDlg.cpp
Method LRESULT CemuleDlg::OnWebGUIInteraction(WPARAM wParam, LPARAM lParam)
if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &hToken)) throw;
Throwing exception in that form should be used in catch() blocks, because it re-throws an existing exception.
Therefore at least something should be thrown; preferably of the class std::exception. In the very least, throw 0; might be used.
3. File IESecurity.cpp
Method STDMETHODIMP CMuleBrowserControlSite::XInternetSecurityManager::MapUrlToZone
TRACE(_T("%hs: URL=%ls, Zone=%d, Flags=0x%x\n"), "MapUrlToZone", pwszUrl, *pdwZone, dwFlags); if (pdwZone != NULL) {
It would be safer to move TRACE() down after if() line; or change the line to:
TRACE(_T("%hs: URL=%ls, Zone=%d, Flags=0x%x\n"), "MapUrlToZone", pwszUrl, pdwZone ? *pdwZone : 0, dwFlags);
4. File IrcMain.cpp
Method void CIrcMain::ParseMessage(CString sRawMessage)
sSound.Format(_T("%sSounds\\IRC\\%s"), thePrefs.GetMuleDirectory(EMULE_EXECUTEABLEDIR), sSound);
Formatting a string into itself is at least questionable, simple concatenation would be safer:
sSound = thePrefs.GetMuleDirectory(EMULE_EXECUTEABLEDIR)+_T("Sounds\\IRC\\")+sSound;
This post has been edited by fox88: 27 June 2015 - 03:59 PM
#407
Posted 02 May 2015 - 10:37 PM
Then I attached debugger to look at it, then tried to continue, but eMule crashed in void CListenSocket::OnAccept(int nErrorCode) while executing code
CUpDownClient* pClient = theApp.clientlist->FindClientByIP(SockAddr.sin_addr.S_un.S_addr); AddDebugLogLine(false, _T("Rejecting connection attempt of banned client %s %s"), ipstr(SockAddr.sin_addr.S_un.S_addr), pClient->DbgGetClientInfo());
Obviously, the client already disconnected and pClient is null.
Checking for null before callig AddDebugLogLine() would be appropriate here.
#408
Posted 09 May 2015 - 01:00 PM
method void CWebSocket::OnRequestReceived(char* pHeader, DWORD dwHeaderLen, char* pData, DWORD dwDataLen, in_addr inad)
bool filereq=false; ... if (sURL.GetLength()>4 && // min length (for valid extentions) (sURL.Right(4).MakeLower()==".gif" || sURL.Right(4).MakeLower()==".jpg" || sURL.Right(4).MakeLower()==".png" || sURL.Right(4).MakeLower()==".ico" ||sURL.Right(4).MakeLower()==".css" ||sURL.Right(3).MakeLower()==".js" || sURL.Right(4).MakeLower()==".bmp" || sURL.Right(5).MakeLower()==".jpeg" ) && sURL.Find("..")==-1 // dont allow leaving the emule-webserver-folder for accessing files ) filereq=true;
There is a bug: check for length above 4 yet trying to look for 3 character ".js". Besides, that string is also a valid name in Windows.
It could be rewritten as:
bool filereq = sURL.GetLength()>=3 && sURL.Find("..")==-1; // don't allow leaving the emule-webserver-folder for accessing files if (filereq) { CStringA ext(sURL.Right(5).MakeLower()); int i = ext.ReverseFind('.'); ext.Delete(0, i); filereq = i>=0 && ext.GetLength()>2 && (ext==".gif" || ext==".jpg" || ext==".png" || ext==".ico" || ext==".css" || ext==".bmp" || ext==".js" || ext==".jpeg"); }
Obviously, the previous definition of filereq should be removed.
This post has been edited by fox88: 27 June 2015 - 03:09 PM
#409
Posted 09 May 2015 - 07:04 PM
_ftprintf(fp, _T("%s %i %hs"), szTime, reportType, message);
Since
_ftprintf(fp, _T("%ls %i %hs"), szTime, reportType, message);
Edit. I found a few %hs formatting codes around.
According to www.securecoding.cert.org, it should not be used.
Edit 2. %hs returned back into MS online documentation as supported in VS2013.
This post has been edited by fox88: 27 June 2015 - 03:11 PM
#410
Posted 10 May 2015 - 10:59 AM
(void)m_strComment;
Most certainly it was intended to be
m_strComment.Empty();
Edit. There are a few such casts in the following block. Compiler omits these code lines even in debug build.
My guess it was prepared for assert checks, but remained unused.
This post has been edited by fox88: 31 May 2015 - 12:27 PM
#411
Posted 10 May 2015 - 12:18 PM
Windows does not care if returned message is null terminated.
The simplest way to fix might be to add function into OtherFunctions.cpp:
BOOL GetExceptionMessage(const CException& ex, LPTSTR lpszError, UINT nMaxError) { BOOL ret = ex.GetErrorMessage(lpszError, nMaxError); if (lpszError) lpszError[ret && nMaxError ? nMaxError-1 : 0] = 0; return ret; }
MS recommends to use constant references in exception handlind, like
catch(const Exeption& ex) {...}. That was the reason for the first parameter declaration.
PS. At first I thought there are lots of files with that problem. Luckily, there were calls to the function with the same name as the method in question. Still about two dozen, usually with several entries.
This post has been edited by fox88: 10 May 2015 - 06:41 PM
#412
Posted 10 May 2015 - 06:57 PM
fox88, on 11 October 2014 - 05:40 PM, said:
It was interesting to learn that eMule has special setting for that:
m_bAdjustNTFSDaylightFileTime=ini.GetBool(L"AdjustNTFSDaylightFileTime", true);
So by default eMule adjusts file time, which initially was time zone independent?
I see no reason for that static adjustment.
Obviously every DST on/off event or time zone information change will force rehashing of all your files.
If we ever need to show file timestamp, the adjustment should be done on the fly, because time shift might happen while eMule runs, and that would invalidae all previous calculations.
Edit. On second thought, I could guess why that happened.
File Times
FAT uses local time, and for that reason eMule needed to adjust time in attempt to avoid hashing.
The default global AdjustNTFSDaylightFileTime=true setting should bs fine for FAT volumes, but might be incorrect for NTFS volumes.
Also I read comments in code and found Time stamp changes with daylight savings. Time in known.met needs no adjustment as I see it.
This post has been edited by fox88: 31 May 2015 - 12:52 PM
#413
Posted 11 May 2015 - 01:03 PM
Some Support, on 22 March 2014 - 01:35 AM, said:
Remember that old message?
Now there is a good reason to return to the subject of calling constructors instead of methods (bug found using code analyzer).
One of several cases in file ClientVersionInfo.h:
CClientVersionInfo(uint32 dwTagVersionInfo, UINT nClientMajor) { UINT nClientMajVersion = (dwTagVersionInfo >> 17) & 0x7f; UINT nClientMinVersion = (dwTagVersionInfo>> 10) & 0x7f; UINT nClientUpVersion = (dwTagVersionInfo >> 7) & 0x07; CClientVersionInfo(nClientMajVersion, nClientMinVersion, nClientUpVersion, (UINT)CVI_IGNORED, nClientMajor, SO_UNKNOWN); }
Constructor is called, new object is created and immediately destroyed. No assignment to the fields in the current object done.
While it is possible to call constructor as a method, it is considered as bad practice - and potentially dangerous.
A better way is to define privite initializer function, and the code could be rewritten about like this:
public: CClientVersionInfo(CString strPCEncodedVersion) { init((UINT)CVI_IGNORED, (UINT)CVI_IGNORED, (UINT)CVI_IGNORED, (UINT)CVI_IGNORED, SO_UNKNOWN, SO_UNKNOWN); ... CClientVersionInfo(uint32 dwTagVersionInfo, UINT nClientMajor) { UINT nClientMajVersion = (dwTagVersionInfo >> 17) & 0x7f; UINT nClientMinVersion = (dwTagVersionInfo>> 10) & 0x7f; UINT nClientUpVersion = (dwTagVersionInfo >> 7) & 0x07; init(nClientMajVersion, nClientMinVersion, nClientUpVersion, (UINT)CVI_IGNORED, nClientMajor, SO_UNKNOWN); } CClientVersionInfo(uint32 nVerMajor, uint32 nVerMinor, uint32 nVerUpdate, uint32 nVerBuild, uint32 ClientTypeMajor, uint32 ClientTypeMinor = SO_UNKNOWN) { init(nVerMajor, nVerMinor, nVerUpdate, nVerBuild, ClientTypeMajor, ClientTypeMinor); } CClientVersionInfo() { init((UINT)CVI_IGNORED, (UINT)CVI_IGNORED, (UINT)CVI_IGNORED, (UINT)CVI_IGNORED, SO_UNKNOWN, SO_UNKNOWN); } CClientVersionInfo(const CClientVersionInfo& cv) { *this = cv; } CClientVersionInfo& operator=(const CClientVersionInfo& cv) { init(cv.m_nVerMajor, cv.m_nVerMinor, cv.m_nVerUpdate, cv.m_nVerBuild, cv.m_ClientTypeMajor, cv.m_ClientTypeMinor); return *this; } ... private: void init(uint32 nVerMajor, uint32 nVerMinor, uint32 nVerUpdate, uint32 nVerBuild, uint32 ClientTypeMajor, uint32 ClientTypeMinor) { m_nVerMajor = nVerMajor; m_nVerMinor = nVerMinor; m_nVerUpdate = nVerUpdate; m_nVerBuild = nVerBuild; m_ClientTypeMajor = ClientTypeMajor; m_ClientTypeMinor = ClientTypeMinor; }
This post has been edited by fox88: 27 June 2015 - 03:03 PM
#414
Posted 11 May 2015 - 03:45 PM
Some Support, on 25 April 2015 - 06:11 PM, said:
I used to set 1024KB/s limit with 10Mbit/s line; and had perfectly flat line in upload graph.
With the latest change it is very uneven upload.
Right now: there are only 20 slots, and about half of the slots accept data at rates below 10KB/s.
Consequently there are frequent drops in the graph; commonly to 800KB/s, sometimes to 400 and below - recent one was to approximately 250.
Average speed is about 950KB/s only.
The conclusion: current algorithm with small fixed number of slots behaves inadequately.
Edit.
Decreased slot limit to 25*1024. The upload graph again is very smooth horizontal line.
This post has been edited by fox88: 13 May 2015 - 07:10 PM
#415
Posted 12 May 2015 - 10:31 PM
Here is an edited version with removed unnecessary variables and check for errors or end of file when reading file:
bool gotostring(CFile &file, const uchar *find, LONGLONG plen) { LONGLONG j = 0; ULONGLONG len = file.GetLength(); uchar temp; for (ULONGLONG i = file.GetPosition(); i < len && file.Read(&temp, 1) == 1; i++) { if (temp == find[j]) j++; else if (temp == find[0]) j = 1; else j = 0; if (j >= plen) return true; } return false; }
File CBase64Coding.cpp, method BOOL CBase64Coding::Encode(const char * source, int len, char * destination_string) contains extra lines, unused by eMule.
Declaration of number_of_bytes_encoded, initialized with zero is enough; the assignments are not needed any more:
BYTE byte_3 = 0; /* DWORD number_of_bytes_encoded = (DWORD) ( (double) number_of_bytes_to_encode / (double) 0.75 ) + 1; // Now add in the CR/LF pairs, each line is truncated at 72 characters // 2000-05-12 // Thanks go to Ilia Golubev (ilia@varicom.co.il) for finding a bug here. // I was using number_of_bytes_to_encode rather than number_of_bytes_encoded. number_of_bytes_encoded += (DWORD)( ( ( number_of_bytes_encoded / BASE64_NUMBER_OF_CHARACTERS_PER_LINE ) + 1 ) * 2 ); */ DWORD number_of_bytes_encoded = 0; char * destination = destination_string;
A numbe of bugs with unsigned values.
File resizablelayout.cpp, method BOOL CResizableLayout::NeedsRefresh(const CResizableLayout::LayoutInfo& layout, const CRect& rectOld, const CRect& rectNew)
Line
info.nMax -= __max(info.nPage-1, 0);
should be
if (info.nPage > 1) info.nMax -= info.nPage-1;
File tag_render.cpp, method size_t ID3_TagImpl::PaddingSize(size_t curSize) const
Line
if ((this->GetPrependedBytes()-ID3_TagHeader::SIZE > 0) &&
should be
if ((this->GetPrependedBytes() > ID3_TagHeader::SIZE) &&
File EncryptedStreamSocket.cpp, method int CEncryptedStreamSocket::SendNegotiatingData(const void* lpBuf, uint32 nBufLen, uint32 nStartCryptFromByte, bool bDelaySend)
Line
if (nBufLen - nStartCryptFromByte > 0)
should be
if (nBufLen > nStartCryptFromByte)
File CorruptionBlackBox.cpp, method
void CCorruptionBlackBox::TransferredData(uint64 nStartPos, uint64 nEndPos, const CUpDownClient* pSender)
two asserts should be fixed
ASSERT( nRelEndPos - m_aaRecords[nPart][i].m_nStartPos > 0); ... ASSERT( m_aaRecords[nPart][i].m_nEndPos - nRelStartPos > 0);
as
ASSERT(nRelEndPos > m_aaRecords[nPart][i].m_nStartPos); ... ASSERT(m_aaRecords[nPart][i].m_nEndPos > nRelStartPos);
And one more bug in ResizableLib
There must be logical and operation instead of logical or.
This post has been edited by fox88: 18 July 2015 - 10:57 AM
#416
Posted 13 May 2015 - 07:39 AM
fox88, on 11 May 2015 - 04:45 PM, said:
Some Support, on 25 April 2015 - 06:11 PM, said:
I used to set 1024KB/s limit with 10Mbit/s line; and had perfetly flat line in upload graph.
With the latest change it is very uneven upload.
Right now: there are only 20 slots, and about half of the slots accept data at rates below 10KB/s.
Consequently there are frequent drops in the graph; commonly to 800KB/s, sometimes to 400 and below - recent one was to approximately 250.
Average speed is about 950KB/s only.
The conclusion: current algorithm with small fixed number of slots behaves inadequately.
Edit.
Decreased slot limit to 25*1024. The upload graph again is very smooth horizontal line.
Thanks for the report. It seems to be rather a bug in the uploadcode than a problem with the intention itself. At 1024KB/s there should be more slots opened than 20 and even with 20 slots it's unlikely that they wouldn't be able to fill 10MBit. I'll look into that. Did you set a capacity and a limit or did you use USS?
#417
Posted 13 May 2015 - 07:16 PM
Some Support, on 13 May 2015 - 10:39 AM, said:
Capacity is set at 1200.
I tried to use USS previously, not with the latest changes; and I don't think there were any downloads.
At first it was quite good, upload speed went up by 100 KB/s, if not more.
But about two days later I check in the morning, and upload is steady at 700 KB/s only.
So I reverted to static limit again.
Now with USS speed is 1151-1160KB/s. I'll watch how it behaves.
This post has been edited by fox88: 13 May 2015 - 09:22 PM
#418
Posted 16 May 2015 - 01:58 PM
In the evenings it was fine when upload was above 1000 KB/s, sometimes very close to the limit 1200.
The line is smooth; not straight as a ruler, but close to that and definitely nothing like sawtooth pattern.
But in the mornings, when downloaders are less numerous, things get worse.
One time it was below 200, and in 40 minutes grew up to ~400 only.
Next time upload limit was so low, that there were only 4 slots active, and ~50 peer in queue.
Both times setting static limit soon increased upload speed to the usual value of 1024.
As far as I heard, USS checks ping to selected IP(s) and tries to adjust upload limit accorgingly.
In morning time there are less peers, and upload might drop rather low even with static limit.
I believe USS "thinks" that speed drops as a result of high network load and decreases the limit (sometimes too much and without any real necessity).
#419
Posted 16 May 2015 - 02:43 PM
fox88, on 16 May 2015 - 02:58 PM, said:
In the evenings it was fine when upload was above 1000 KB/s, sometimes very close to the limit 1200.
The line is smooth; not straight as a ruler, but close to that and definitely nothing like sawtooth pattern.
But in the mornings, when downloaders are less numerous, things get worse.
One time it was below 200, and in 40 minutes grew up to ~400 only.
Next time upload limit was so low, that there were only 4 slots active, and ~50 peer in queue.
Both times setting static limit soon increased upload speed to the usual value of 1024.
As far as I heard, USS checks ping to selected IP(s) and tries to adjust upload limit accorgingly.
In morning time there are less peers, and upload might drop rather low even with static limit.
I believe USS "thinks" that speed drops as a result of high network load and decreases the limit (sometimes too much and without any real necessity).
Is this behaviour new in build 16 or does it happen with other versions too?
#420
Posted 16 May 2015 - 03:24 PM
Some Support, on 16 May 2015 - 05:43 PM, said:
As I mentioned earlier, drop to 700 happened a while ago, before the latest changes.
Edit. That was with 3 KB/s slots.
Now I keep it down to 25 KB/s, not 50.
This post has been edited by fox88: 16 May 2015 - 05:28 PM