aMule Forum
English => en_Bugs => Topic started by: visualage on December 28, 2007, 01:24:35 PM
-
In aMuleCVS-20071226, when the file is large enough, amuled will throw an uncatched exception when seeking the file (saying position is negative something). The following patch will fix the problem.
--- amule-cvs.org/src/PartFile.cpp 2007-12-19 03:54:18.000000000 +0800
+++ amule-cvs/src/PartFile.cpp 2007-12-28 18:40:14.250445872 +0800
@@ -2357,8 +2357,9 @@ void CPartFile::Delete()
}
-bool CPartFile::HashSinglePart(uint16 partnumber)
+bool CPartFile::HashSinglePart(uint16 pn)
{
+ uint64 partnumber = pn;
if ((GetHashCount() <= partnumber) && (GetPartCount() > 1)) {
AddLogLineM(true,
CFormat( _("Warning: Unable to hash downloaded part - hashset incomplete for '%s'") )
-
Some more detail about this error:
Basically, in HashSinglePart(sint16 partnumber), PARTSIZE * partnumber might be overview in sint16 or sint32. Therefore, it is better to widen the partnumber to uint64 at the first statement of HashSinglePart().
-
There might be a simpler fix. Just change the parameter of HashSinglePart from uint16 to uint64.
-
Hi visualage,
I agree with the problem, but your solution is using a side effect to fix it, and that does not help with readability and cleareness. I would rather create a variable large enough to hold (PARTSIZE * partnumber) the value and use it.
I have implemented this and committed.
Thanks!
-
Hi phoenix,
does this change anything ? PARTSIZE is declared as unsigned long long, so the expression (PARTSIZE * partnumber) should not overflow anyway.
-
Hi Stu,
My fix was the following:
--- trunk/src/PartFile.cpp 2007-12-28 13:42:10 UTC (rev 7855)
+++ trunk/src/PartFile.cpp 2007-12-28 14:14:44 UTC (rev 7856)
@@ -2361,23 +2361,22 @@
} else {
CMD4Hash hashresult;
uint64 length = PARTSIZE;
-
+ const uint64 offset = length * partnumber;
try {
- m_hpartfile.Seek(PARTSIZE * partnumber, wxFromStart);
- if ((uint64)(PARTSIZE * (partnumber + 1)) > m_hpartfile.GetLength()){
- length = (m_hpartfile.GetLength() - (PARTSIZE * partnumber));
+ m_hpartfile.Seek(offset, wxFromStart);
+ if (offset + PARTSIZE > m_hpartfile.GetLength()) {
+ length = m_hpartfile.GetLength() - offset;
wxASSERT( length <= PARTSIZE );
}
-
CreateHashFromFile(&m_hpartfile, length, hashresult.GetHash());
} catch (const CIOFailureException& e) {
AddLogLineM(true, CFormat( wxT("EOF while hashing downloaded part %u with length %u (max %u) of partfile '%s' with length %u: %s"))
- % partnumber % length % ((partnumber*PARTSIZE)+length) % GetFileName() % GetFileSize() % e.what());
+ % partnumber % length % (offset+length) % GetFileName() % GetFileSize() % e.what());
SetPartFileStatus(PS_ERROR);
return false;
} catch (const CEOFException& e) {
AddLogLineM(true, CFormat( wxT("EOF while hashing downloaded part %u with length %u (max %u) of partfile '%s' with length %u: %s"))
- % partnumber % length % ((partnumber*PARTSIZE)+length) % GetFileName() % GetFileSize() % e.what());
+ % partnumber % length % (offset+length) % GetFileName() % GetFileSize() % e.what());
return false;
}
Apparently, PARTSIZE is 32 bits, in spite of beeing declared as "ull". I created a 64 bit constant "offset" to deal with it. But the proper fix is to change PARTFILE to a 64 bit constant value. Do you know from your head the proper way to declare it? I am too lazy these days... :D
EDIT: Hum, there is something strange really, "uul" is 8 bytes. Maybe there is another different declaration of PARTFILE? I will search the code, but probably only next year now...
-
ull should be 64bits indeed. The code was ok :P
-
ull should be 64bits indeed. The code was ok :P
Unfortunately, the code was not OK. There is a second place defining PARTSIZE. I simply fgreped the source code and found:
src/utils/aLinkCreator/src/md4.h:const unsigned int PARTSIZE = 9500*1024;
Obviously, this is a 32-bit integer.
-
You're all wrong. ALC has nothing to do with it. The PARTSIZE declaration is broken I'm afraid:
enum FileConstants {
PARTSIZE = 9728000ull,
BLOCKSIZE = 184320u,
EMBLOCKSIZE = 184320u
};
A simple trial & error:
const uint64 PARTSIZE2 = 9728000;
uint16 qqqq = 0xffff;
AddDebugLogLineM(false, logPartFile, CFormat(wxT("inttest %llu %llu")) % (qqqq * PARTSIZE) % (qqqq * PARTSIZE2));
result:
PartFile.cpp(321): PartFiles: inttest 1869320192 637524480000
It seems enums are limited to 32 bit, whatever type is used for assignment.
Fix: just kick the enum in Constants.h:
const uint64 PARTSIZE = 9728000ull;
const uint32 BLOCKSIZE = 184320u;
const uint32 EMBLOCKSIZE = 184320u;
Cheers, Stu
-
The ALC is wrong and might generate a problem on ALC and friends, but not on aMule.
sturedman is right, excelent work, your fix will be on tomorrow SVN tarball.
Cheers!
-
Funny about enums. Glad you catched that.
-
I'm glad I can help! :D
I googled a bit about the enums, seems another workaround would have been: (http://msdn2.microsoft.com/en-us/library/2dzy4k6e.aspx)
enum FileConstants : uint64 {
PARTSIZE = 9728000ull,
BLOCKSIZE = 184320u,
EMBLOCKSIZE = 184320u
};
(but I haven't tried it).