[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#872334: apt-utils: apt-ftparchive does not correctly cache filesizes for packages > 4GB



On Wed, Aug 16, 2017 at 12:52:36PM +0100, David McBride wrote:
> For example, a package of size 7162161474 bytes will today have the following
> sequence of bytes stored in its cache entry:
> 
> \xaa\xe5\xe9\x42\x00\x00\x00\x00
> 
> (When re-read, this will produce a file-size value of 7162161474 mod 32bits,
> i.e. 2867194178.)
> 
> With this patch applied, apt-ftparchive will correctly store this entry:
> 
> \x00\x00\x00\x01\xaa\xe5\xe9\x42

OK, so I'd say that maybe the optimal solution then is to store it as:

\xaa\xe5\xe9\x42\x00\x00\x00\x01

That is, swap the upper and lower 4 bytes when loading/writing. This
way, files writing by old versions behave as before; and for old code,
databases generated by new code will look the same for small files - that
is, you can also rollback your apt-ftparchive version without anything
breaking.

> -   CurStat.mtime = htonl(St.st_mtime);
> +   CurStat.mtime = htobe32(St.st_mtime);

On the other hand, we should really migrate mtime to 64-bit
as well.

But: The question is do we want to just change mtime to a 64-bit
field, or do we just want to append a new integer to it, and manually
reconstruct the 64-bit? Then we can avoid changing the file format
and old versions can still read it (until 2038).


>     CurStat.Flags |= FlSize;
>     
>     return true;
> @@ -263,8 +263,8 @@ bool CacheDB::GetCurStat()
>           return _error->Error("Cache record size mismatch (%ul)", Data.size);
>        }
>  
> -      CurStat.Flags = ntohl(CurStat.Flags);
> -      CurStat.FileSize = ntohl(CurStat.FileSize);
> +      CurStat.Flags = be32toh(CurStat.Flags);
> +      CurStat.FileSize = be64toh(CurStat.FileSize);

So here, do

	auto tmp = b64toh(CurStat.FileSize);
	CurStat.FileSize = (tmp & 0xFFFF << 32) | (tmp >> 32 & 0xFFFF)

(precedence might be wrong, but I think the idea is clear)


>     }      
>     return true;
>  }
> @@ -524,12 +524,12 @@ bool CacheDB::Finish()
>        return true;
>  
>     // Write the stat information
> -   CurStat.Flags = htonl(CurStat.Flags);
> -   CurStat.FileSize = htonl(CurStat.FileSize);
> +   CurStat.Flags = htobe32(CurStat.Flags);
> +   CurStat.FileSize = htobe64(CurStat.FileSize);

Same as above, basically

>     InitQueryStats();
>     Put(&CurStat,sizeof(CurStat));
> -   CurStat.Flags = ntohl(CurStat.Flags);
> -   CurStat.FileSize = ntohl(CurStat.FileSize);
> +   CurStat.Flags = be32toh(CurStat.Flags);
> +   CurStat.FileSize = be64toh(CurStat.FileSize);

Same here



-- 
Debian Developer - deb.li/jak | jak-linux.org - free software dev
                  |  Ubuntu Core Developer |
When replying, only quote what is necessary, and write each reply
directly below the part(s) it pertains to ('inline').  Thank you.


Reply to: