Control: tags -1 + pending

2016-09-18 12:09 David Kalnischkies:
On Sat, Sep 17, 2016 at 10:32:53PM -0400, Aaron M. Ucko wrote:
> On a first glance, using the module operator rather than bitwise
> operations looks a bit odd.

It's unconventional, but should optimize to bitwise masking in practice
because the modulus is a constant power of two.  Using an explicit mask
would be cleaner, but AFAICT apt's headers don't predefine one.

From the apt side I can confirm that I 'recently' added new flags
(specifically in 8c7af4d4 on Thu Jul 16 11:15:25 2015).

After discussing this a bit more with David, I decided to change a bit
the patch and use 0xf as mask, because the enum documents that only the
lower 4 bits are to be used as an operator.

In practice the %-operation would probably achieve the same, but I think
that this way is easier to understand.

Patch attached.  I will try to get it into the next release.

Aaron, it would be great if you could test it and check that it fixes
the problem.  I never stumbled upon this problem, even if I use
multi-arch in most of my systems.


Thanks.
--
Manuel A. Fernandez Montecelo <manuel.montez...@gmail.com>
>From 867141bbd4c2ba26c7c3f86923b7177958ac3d9b Mon Sep 17 00:00:00 2001
From: "Manuel A. Fernandez Montecelo" <manuel.montez...@gmail.com>
Date: Thu, 16 Feb 2017 23:40:03 +0100
Subject: [PATCH] Use a more strict mask for comparison of dependency operators
 (Closes: #836567, #849370)

Use a more strict mask for comparison of dependency operators, thanks to
Aaron M. Ucko for the initial patch (Closes: #836567, #849370)

Recently apt started to use more values in an enum used for dependency
operator comparisons, which causes some problems because aptitude's code
from many years ago, which did not expect new values.

Using a stricter mask to capture only the operators, as per the
documentation of the enum (lower 4 bits).
---
 NEWS                   | 12 ++++++++++++
 src/generic/apt/apt.cc |  7 +++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index f22ea692..58529ba6 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,18 @@
 [2017-01-xx]
 Version 0.8.6 UNRELEASED
 
+- Bug fixes:
+
+  * Use a more strict mask for comparison of dependency operators, thanks to
+    Aaron M. Ucko for the initial patch (Closes: #836567, #849370)
+
+    Recently apt started to use more values in an enum used for dependency
+    operator comparisons, which causes some problems because aptitude's code
+    from many years ago, which did not expect new values.
+
+    Using a stricter mask to capture only the operators, as per the
+    documentation of the enum (lower 4 bits).
+
 - Translation updates:
 
   * ru.po: Russian translation by Lev Lamberov (Closes: #855329)
diff --git a/src/generic/apt/apt.cc b/src/generic/apt/apt.cc
index 1850825d..68c0e49c 100644
--- a/src/generic/apt/apt.cc
+++ b/src/generic/apt/apt.cc
@@ -1141,8 +1141,11 @@ static bool subsumes(const pkgCache::DepIterator &d1,
       if(!d2.TargetVer())
 	return false;
 
-      pkgCache::Dep::DepCompareOp t1 = (pkgCache::Dep::DepCompareOp) (d1->CompareOp &~ pkgCache::Dep::Or);
-      pkgCache::Dep::DepCompareOp t2 = (pkgCache::Dep::DepCompareOp) (d2->CompareOp &~ pkgCache::Dep::Or);
+      // the lower 4 bits are the actual operator (from documentation of the
+      // data type)
+      int comp_mask = 0xf;
+      pkgCache::Dep::DepCompareOp t1 = (pkgCache::Dep::DepCompareOp) (d1->CompareOp & comp_mask);
+      pkgCache::Dep::DepCompareOp t2 = (pkgCache::Dep::DepCompareOp) (d2->CompareOp & comp_mask);
 
       int cmpresult = _system->VS->DoCmpVersion(d1.TargetVer(), d1.TargetVer()+strlen(d1.TargetVer()),
 						d2.TargetVer(), d2.TargetVer()+strlen(d2.TargetVer()));
-- 
2.11.0

Reply via email to