Ticket #64 (closed defect)

Opened 9 years ago

Last modified 9 years ago

sftp 4GB stat problem

Reported by: mikeprotts Owned by: bagder
Priority: normal Milestone:
Component: SFTP Version:
Keywords: Cc: mikeprotts, bagder
Blocked By: Blocks:

Description

I have been working with libssh2 and curl and found a problem with the stat of a remote file that is larger than 4GB (curl uses this to decide when the transfer has finished). The attr size value returned from libssh2_sftp_stat is losing the hig bytes, therefore returning a size - 4GB instead of size.

The problem seems to be in misc.c function libssh2_ntohu64 where the return value is being calculated, and I think the compiler is performing calculations as 32 it instead of 64. I have a work around (below) which seems ok - comments welcome.

libssh2_uint64_t
libssh2_ntohu64(const unsigned char *buf)
{

unsigned long lsl, msl;
libssh2_uint64_t aval64; /* Used for return value */


msl = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
lsl = (buf[4] << 24) | (buf[5] << 16) | (buf[6] << 8) | buf[7];
aval64 = msl; /* No cast needed as 32 to 64 bit should work */
aval64 = aval64 * 65536 * 65536; /* now force to correct value */
aval64 = aval64 + lsl; /* add 32 to 64 bit should be ok */
return aval64; /*was ((msl * 65536) * 65536) + lsl;*/

}

In case this is thought to be a compiler issue, the compiler information
+follows:
mikep@rockwell:~/src/rexxcurl/RexxCURL-2.0$ uname -a
Linux rockwell 2.6.18-5-686 #1 SMP Wed Sep 26 17:54:59 UTC 2007 i686 GNU/Linux
mikep@rockwell:~/src/rexxcurl/RexxCURL-2.0$ gcc -v
Using built-in specs.
Target: i486-linux-gnu
Configured with: ../src/configure -v
+--enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr
+--enable-shared --with-system-zlib --libexecdir=/usr/lib
+--without-included-gettext --enable-threads=posix --enable-nls
+--program-suffix=-4.1 --enable-cxa_atexit --enable-clocale=gnu
+--enable-libstdcxx-debug --enable-mpfr --with-tune=i686
+--enable-checking=release i486-linux-gnu
Thread model: posix
gcc version 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)

--
Regards

Mike Protts
Senior Technical Consultant
Pro:Atria Ltd
+44(0) 870 7656453

mikep@…

Change History

comment:1 Changed 9 years ago by bagder

The problem is that the calculation is done with only 32bit values, so at least one of them need to be typecasted to a 64bit version. How about this patch (much smaller):

--- misc.c 6 Aug 2007 20:48:06 -0000 1.19
+++ misc.c 6 Mar 2008 12:50:09 -0000
@@ -62,7 +62,7 @@

msl = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
lsl = (buf[4] << 24) | (buf[5] << 16) | (buf[6] << 8) | buf[7];


  • return ((msl * 65536) * 65536) + lsl;

+ return (((libssh2_uint64_t)msl * 65536) * 65536) + lsl;

}

Although I would prefer to clean this up while at it, and intead do the following patch - would be cool if you could verify if this works fine too:

--- misc.c 6 Aug 2007 20:48:06 -0000 1.19
+++ misc.c 6 Mar 2008 12:51:44 -0000
@@ -62,7 +62,7 @@

msl = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
lsl = (buf[4] << 24) | (buf[5] << 16) | (buf[6] << 8) | buf[7];


  • return ((msl * 65536) * 65536) + lsl;

+ return (((libssh2_uint64_t)msl <<32) | lsl;

}


comment:2 Changed 9 years ago by mikeprotts

I'll try both and report back.

Cheers
Mike

comment:3 Changed 9 years ago by mikeprotts

Both patches seem to work (apart from the extra '(' in the second), so I am now running some more tests. I will also look at other platforms to see if the build is OK.

Cheers
Mike

comment:4 Changed 9 years ago by mikeprotts

Having confirmed the stat is returng correct data I've also completed 4.5GB download (one using curl command line and one using libcurl). This was using:
return ((libssh2_uint64_t)msl <<32) | lsl;

Cheers
Mike

comment:5 Changed 9 years ago by bagder

Great! I've now committed this fix to CVS. Case closed!

Note: See TracTickets for help on using tickets.