[Avi-3k]

hi everybody

i was going through the code for some stuff for Skinner when i noticed
something strange. eMule set's it's dir mode in the current user's hkey:

void CPreferences::ChangeUserDirMode(int nNewMode){
...
	// 0 = Multiuser, 1 = Publicuser, 2 = ExecuteableDir.
	CRegKey rkEMuleRegKey;
	if (rkEMuleRegKey.Create(HKEY_CURRENT_USER, _T("Software\\eMule")) == ERROR_SUCCESS){
		if (rkEMuleRegKey.SetDWORDValue(_T("UsePublicUserDirectories"), nNewMode) != ERROR_SUCCESS)
			DebugLogError(_T("Failed to write registry key to switch UserDirMode"));
		else
			m_nCurrentUserDirMode = nNewMode;
		rkEMuleRegKey.Close();
	}

shouldn't it be in the local machine key?
this is so the setting will be unified for all the users on the same computer.


second possible bug, i've started eMule 0.49b in debug (compiled myself without any changes),
when i tried to change the upload speed in the prefs, it crashed because of division by zero in
void COScopeCtrl::SetRange() at

	m_PlotData[iTrend].dVerticalFactor = (double)m_nPlotHeight / m_PlotData[iTrend].dRange;

this was because the capacity was zero (from what i've seen).
there should be a check for .dRange to make sure it's not zero...


third possible bug, i've shared some files with 0.49b, including it's release files (bin & installer)
after sharing them, i tried to download them but eMule didn't notify me that i share them
because i've pasted the links, then i checked the code and saw that
void CDownloadQueue::AddFileLinkToDownload() doesn't check if the file is known/shared...:

void CDownloadQueue::AddFileLinkToDownload(CED2KFileLink* pLink, int cat)
{
 if (IsFileExisting(pLink->GetHashKey(), true))
		return;
 ....
}

also, some code improvements:
in SafeFile.cpp, the CFileDataIO::Write[Long]String() function has 2 separate conditions
to write the string in multibyte, which can be merged... instead of:

	if (eEncode == utf8strRaw)
		//utf8
	else if (eEncode == utf8strOptBOM)
	{
		if (NeedUTF8String(rstr))
			//bom-utf8
		else
			//multibyte
	}
	else
		//multibyte

can be changed to:

	if (eEncode == utf8strRaw)
		//utf8
	else if (eEncode == utf8strOptBOM && NeedUTF8String(rstr))
		//bom-utf8
	else
		//multibyte

i'll update the thread soon, i forgot to write all the things i saw
and i may have forgotten something...

other improvements and fixes are in my sig, some are really old and should be taken care of
(like TitleMenu dll loading, the BtnSt and other unused classes, etc...)

Avi3k

[Tuxman]

Avi-3k, on Aug 2 2008, 11:58 PM, said:

shouldn't it be in the local machine key?
this is so the setting will be unified for all the users on the same computer.

I thought this was intended as it was changed quite recently... couldn't find the matching entry in the change log, though...

[Avi-3k]

@Tuxman, me too, that's why i'm asking
(i actually saw this was this way in v0.49a after checking 0.49b)


a bug fix in otherfunctions.cpp in StringLimit(), it return (length+3) chars instead of (length) chars
should be

CString StringLimit(CString in, UINT length)
{
	if ((UINT)in.GetLength() <= length || length < 10)
		return in;
	return (in.Left(length-8) + _T("...") + in.Right(5)); // changed Right(8) to Right(5)
}

anyway, more improvements to TitleMenu.cpp...
because of Vista you've added a change to use HBITMAP
to draw the icons in menues, but since Vista already supports GDI+
(which was used in a disabled code), why not just use CImage class of ATL/MFC:

CImage img;
ASSERT( img.Create(cx, cy, 32, CImage::createAlphaChannel) );
DrawIconEx(img.GetDC(), 0, 0, hIcon, cx, cy, 0, NULL, DI_NORMAL);
img.ReleaseDC();
HBITMAP hBmp = img.Detach();

(this is not the complete code and was not tested, just an idea on how to... i used a similar code in Skinner)


second improvement is in StringConversion.h/cpp, there're 3 class definitions
with same members and same functions (and they're used the same in SafeFile.h/cpp)
why not use a base class and handle just the c'tors in the derived classes?


third & fourth improvements are in CTag class in packet.h/cpp
3. the union defines a pointer to CString to handle strings, but
all changes to the string include reallocation of the string
and since CString uses a pointer as well, wouldn't it be easier to use TCHAR* instead?
i've tested this in hebMule-Onyx (my fake check feature) and it reduced the memory quite a lot.
the required changes are minimal as well, it's an easy change & a big improvement!

4. bool CTag::WriteNewEd2kTag() can be changed when writing strings
(similar to the code changes to CFileDataIO::Write[Long]String() that are suggested above)


fifth improvement is in otherfunctions.cpp in GetAppImageListColorFlag(),
we can just return the right ILC_ const instead of setting a var and return it in the end


sixth & seventh improvements are in Preferences.cpp...
6. to use a random port eMule has 2 almost identical function CPreferences::GetRandomTCPPort()
& CPreferences::GetRandomUDPPort(), why not create a function that returns a port
and it's parameter is which port table to check (TCP / UDP)? it's less overhead then 2 very similar functions...

7. in CPreferences::CanFSHandleLargeFiles() eMule check both incoming folder & temp folders
but instead of

bool CPreferences::CanFSHandleLargeFiles()	{
	bool bResult = false;
	for (int i = 0; i != tempdir.GetCount(); i++){
		if (!IsFileOnFATVolume(tempdir.GetAt(i))){
			bResult = true;
			break;
		}
	}
	return bResult && !IsFileOnFATVolume(GetMuleDirectory(EMULE_INCOMINGDIR));
}

a better way is to check the incoming first...:

bool CPreferences::CanFSHandleLargeFiles()	{
	if (IsFileOnFATVolume(GetMuleDirectory(EMULE_INCOMINGDIR)))
			return false;
	for (int i = 0; i != tempdir.GetCount(); i++){
		if (!IsFileOnFATVolume(tempdir.GetAt(i)))
			return true;
	}
	return false;
}

i'll update the thread if needed...

Regards,
Avi3k

[Some Support]

Avi-3k, on Aug 2 2008, 09:58 PM, said:

shouldn't it be in the local machine key?
this is so the setting will be unified for all the users on the same computer.

No this is intended and didn't changed recently. There is no reason to enforce a unified setting - if one user on the system wants to have his person configuration, why not. More important a useraccount in general has no write access to the Local Machine branch

[Avi-3k]

ok, i wasn't sure why you intended it this way...

another improvement i saw is in mdump.h/cpp,
since the class is a singleton (or meant to be one)
why not make the c'tor private and use the class name to enable the crash report...
that's what i did after i copied the code to Skinner...

Regards,
Avi3k

[Enig123]

Avi-3k, would you please demonstrate a detailed code of the third improvements in CTag class in packet.h/cpp?

[Avi-3k]

if you've downloaded my alpha of onyx, it's similar to the changes in CSimpleTag.
since i haven't worked on onyx in a while (since the 0.49b betas),
i didn't actually coded anything above, just wrote/pseudoed the code...

after i change the above in onyx, gonna post it here (maybe in code-snippets subforum)

Regards,
Avi3k

[Enig123]

Avi-3k, thanks for the tip.

There's one more question, I'm not very sure about the correspond changes in CTag::WriteNewEd2kTag().

Is it correct to change

Quote

CUnicodeToUTF8 utf8(*m_pstrVal);

to

Quote

CUnicodeToUTF8 utf8(m_pstrVal);

which is the one that I'm not so sure.

This post has been edited by Enig123: 04 August 2008 - 11:58 AM

[Avi-3k]

yes, since m_pstrVal is now of type TCHAR*, the value of m_pstrVal
is the address of the string, which will be casted to the right type for
the c'tors of the classes C*ToUTF8...

Avi3k

[Enig123]

I've tried as Avi-3k told me. But there's some trouble in search with kad after doing that. Since kad part is beyond my ability, I wonder if Avi-3k can dig a little more.

Search with kad result in very little returns, and the log line have something like this:

Quote

2008-8-5 10:43:44: Client UDP socket: prot=0xe5 opcode=0x3b sizeaftercrypt=4019 realsize=4035 Unknown exception: 87.19.116.xxx:45637

which may be what the trouble came from.

[Avi-3k]

it might depend on the nodes you have and your distances,
the changes above do not concern Kad tag code (implemented elsewhere)

Avi3k

[Avi-3k]

here's the code change for CTag...:
packets.h

  const	CString GetStr() const	{ ASSERT(IsStr());		return m_pstrVal; } // Avi3k: improve code
...
 union {
  TCHAR*	m_pstrVal; // Avi3k: improve code
 ...
 };
};

packets.cpp
everywhere there's

m_pstrVal = new CString(pszVal);

change to

m_pstrVal = _tcsdup(pszVal);

(or any other string/function beside pszVal)
and when deleting the pointer, use

delete[] m_pstrVal

because now it's an array
and

bool CTag::WriteNewEd2kTag(...)
{
 ...
	else if (IsStr())
	{
		// Avi3k: improve code
		if (eStrEncode == utf8strRaw)
		{
			CUnicodeToUTF8 utf8(m_pstrVal);
			pstrValA = new CStringA((LPCSTR)utf8, utf8.GetLength());
		}
		else if (eStrEncode == utf8strOptBOM && NeedUTF8String(m_pstrVal))
		{
			CUnicodeToBOMUTF8 bomutf8(m_pstrVal);
			pstrValA = new CStringA((LPCSTR)bomutf8, bomutf8.GetLength());
		}
		else
		{
			CUnicodeToMultiByte mb(m_pstrVal);
			pstrValA = new CStringA((LPCSTR)mb, mb.GetLength());
		}
		// end Avi3k: improve code
...
}

also small changes are required in AbstractFile.h/cpp
(removed reference & since now it's TCHAR* and not CString so it can't be referenced)

	// Avi3k: improve code
	const CString GetStrTagValue(uint8 tagname) const
	...
	const CString GetStrTagValue(LPCSTR tagname) const
	...
	// end Avi3k: improve code

and a small change to SearchFile.cpp (also removed the & reference):

CSearchFile::CSearchFile(CFileDataIO* in_data, bool bOptUTF8, 
						 uint32 nSearchID, uint32 nServerIP, uint16 nServerPort, LPCTSTR pszDirectory, bool bKademlia, bool bServerUDPAnswer)
{
 ...
	const CString rstrFileType = GetStrTagValue(FT_FILETYPE); // Avi3k: improve code
 ...
}

tested the code and it works very well also with Kad.
i've created a test version for this change at http://hebmule.sf.ne.../emule0.49b2.7z
which include some other fixes i suggested and the diff file

Regards,
Avi3k

[Avi-3k]

saw another small thing and thought i'd post it here...
eMule for some reason keeps on saving the downloads.txt file
in its base directory rather then use the config dir, here's the fix:

void CDownloadQueue::ExportPartMetFilesOverview() const
{
	CString strFileListPath = thePrefs.GetMuleDirectory(EMULE_CONFIGDIR) + _T("downloads.txt");
...
}

Regards,
Avi3k

[Tuxman]

Maybe it should also check for "old" downloads.txt files left in the DATABASEDIR then...

[Avi-3k]

yeah, it's something to consider...

btw, another bug i found. if you sort by priority in the download list ctrl,
it doesn't sort the QRs currectly, sometimes full queues & no needed parts users
show between valid on queue users (it's like QR: 10, QR: 14, nnp, fq, QR: 17, etc...)

Avi3k

[dolphinX]

Maybe EMULE_CONFIGBASEDIR is better.

void CDownloadQueue::ExportPartMetFilesOverview() const
{
	CString strFileListPath = thePrefs.GetMuleDirectory(EMULE_CONFIGBASEDIR) + _T("downloads.txt");
...
}