Bug#808381: apt: regressions in CopyFile

[Pino Toscano <pino@debian.org>]

Source: apt
Version: 1.1.5
Severity: important
Tags: patch

Hi,

I identified a couple of regressions in CopyFile:
- the first makes the copy a lot slower than it used to be (copying by
  chunks of 8 bytes)
- the second one breaks CopyFile on the Hurd

Longer explanations of each issue are part of the commit messages of
the patches.

Thanks,
-- 
Pino

>From 457ed97bafa438abbb9ba8fefeb755e6fe450791 Mon Sep 17 00:00:00 2001
From: Pino Toscano <pino@debian.org>
Date: Sat, 19 Dec 2015 12:00:43 +0100
Subject: [PATCH] CopyFile: fix BufSize to a sane value

Commit e977b8b9234ac5db32f2f0ad7e183139b988340d tries to make BufSize
calculated based on the size of the buffer; the problem is that
std::unique_ptr::size() returns a pointer to the data, so sizeof()
equals to the size of a pointer (later divided by sizeof(char), which
is 1). The result is that the CopyFile copies in chunks of 8 bytes,
which is not exactly ideal...

As solution, declare BufSize in advance, and use its value to allocate
the Buf array.
---
 apt-pkg/contrib/fileutl.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index 014f0ee..86ca82c 100644
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -160,8 +160,8 @@ bool CopyFile(FileFd &From,FileFd &To)
       return false;
 
    // Buffered copy between fds
-   std::unique_ptr<unsigned char[]> Buf(new unsigned char[64000]);
-   constexpr unsigned long long BufSize = sizeof(Buf.get())/sizeof(Buf.get()[0]);
+   constexpr size_t BufSize = 64000;
+   std::unique_ptr<unsigned char[]> Buf(new unsigned char[BufSize]);
    unsigned long long ToRead = 0;
    do {
       if (From.Read(Buf.get(),BufSize, &ToRead) == false ||
-- 
2.6.4

>From 09c2114ecacec6d9f91a62fabcb34c05f77ca712 Mon Sep 17 00:00:00 2001
From: Pino Toscano <pino@debian.org>
Date: Sat, 19 Dec 2015 12:06:53 +0100
Subject: [PATCH] CopyFile: avoid failing on EOF on some systems

On EOF, ToRead will be 0, which might trigger on some systems (e.g.
on the Hurd) an error due to the invalid byte count passed to write().
The whole loop already checks for ToRead != 0, so perform the writing
step only when there was actual data read.
---
 apt-pkg/contrib/fileutl.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index 86ca82c..1ffa407 100644
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -165,7 +165,7 @@ bool CopyFile(FileFd &From,FileFd &To)
    unsigned long long ToRead = 0;
    do {
       if (From.Read(Buf.get(),BufSize, &ToRead) == false ||
-	  To.Write(Buf.get(),ToRead) == false)
+	  (ToRead > 0 && To.Write(Buf.get(),ToRead) == false))
 	 return false;
    } while (ToRead != 0);
 
-- 
2.6.4