Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[visualage]

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'") )


[visualage]

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().

[visualage]

There might be a simpler fix. Just change the parameter of HashSinglePart from uint16 to uint64.

[phoenix]

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!

[Stu Redman]

Hi phoenix,

does this change anything ? PARTSIZE is declared as unsigned long long, so the expression  (PARTSIZE * partnumber) should not overflow anyway.

[phoenix]

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...

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...

[Kry]

ull should be 64bits indeed. The code was ok

[visualage]

ull should be 64bits indeed. The code was ok

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.

[Stu Redman]

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

[phoenix]

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!

[Kry]

Funny about enums. Glad you catched that.

[Stu Redman]

I'm glad I can help! 
I googled a bit about the enums, seems another workaround would have been:

Code: [Select]

enum FileConstants : uint64 {
PARTSIZE = 9728000ull,
BLOCKSIZE = 184320u,
EMBLOCKSIZE = 184320u
};(but I haven't tried it).