aMule Forum

English => en_Bugs => Topic started by: visualage on December 28, 2007, 01:24:35 PM

Title: amuled error when file is large enough
Post 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.

Code: [Select]
--- 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'") )
Title: Re: amuled error when file is large enough
Post by: visualage on December 28, 2007, 02:48:15 PM
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().
Title: Re: amuled error when file is large enough
Post by: visualage on December 28, 2007, 02:54:21 PM
There might be a simpler fix. Just change the parameter of HashSinglePart from uint16 to uint64.
Title: Re: amuled error when file is large enough
Post by: phoenix on December 28, 2007, 03:13:34 PM
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!
Title: Re: amuled error when file is large enough
Post by: Stu Redman on December 29, 2007, 10:35:57 PM
Hi phoenix,

does this change anything ? PARTSIZE is declared as unsigned long long, so the expression  (PARTSIZE * partnumber) should not overflow anyway.
Title: Re: amuled error when file is large enough
Post by: phoenix on December 29, 2007, 10:46:33 PM
Hi Stu,

My fix was the following:
Code: [Select]
--- 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...
Title: Re: amuled error when file is large enough
Post by: Kry on December 31, 2007, 12:59:39 PM
ull should be 64bits indeed. The code was ok :P
Title: Re: amuled error when file is large enough
Post by: visualage on January 02, 2008, 10:12:39 AM
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:
Code: [Select]
src/utils/aLinkCreator/src/md4.h:const unsigned int PARTSIZE = 9500*1024;

Obviously, this is a 32-bit integer.
Title: Re: amuled error when file is large enough
Post by: Stu Redman on January 02, 2008, 02:45:11 PM
You're all wrong. ALC has nothing to do with it. The PARTSIZE declaration is broken I'm afraid:
Code: [Select]
enum FileConstants {
PARTSIZE = 9728000ull,
BLOCKSIZE = 184320u,
EMBLOCKSIZE = 184320u
};

A simple trial & error:
Code: [Select]
const uint64 PARTSIZE2 = 9728000;
uint16 qqqq = 0xffff;
AddDebugLogLineM(false, logPartFile, CFormat(wxT("inttest %llu %llu")) % (qqqq * PARTSIZE) % (qqqq * PARTSIZE2));
result:
Code: [Select]
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:
Code: [Select]
const uint64 PARTSIZE = 9728000ull;
const uint32 BLOCKSIZE = 184320u;
const uint32 EMBLOCKSIZE = 184320u;

Cheers, Stu
Title: Re: amuled error when file is large enough
Post by: phoenix on January 02, 2008, 05:48:08 PM
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!
Title: Re: amuled error when file is large enough
Post by: Kry on January 02, 2008, 09:23:49 PM
Funny about enums. Glad you catched that.
Title: Re: amuled error when file is large enough
Post by: Stu Redman on January 02, 2008, 11:11:21 PM
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)
Code: [Select]
enum FileConstants : uint64 {
PARTSIZE = 9728000ull,
BLOCKSIZE = 184320u,
EMBLOCKSIZE = 184320u
};
(but I haven't tried it).