From c7883478da037bffb27ef7cd56c209dd4b34bd69 Mon Sep 17 00:00:00 2001
From: Razvan Becheriu <razvan@isc.org>
Date: Tue, 15 Jun 2021 11:45:57 +0300
Subject: [PATCH] [#1858] addressed comments

---
 src/lib/dhcp/option_data_types.cc | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/lib/dhcp/option_data_types.cc b/src/lib/dhcp/option_data_types.cc
index 9ffa1f6465..6a1c05b8be 100644
--- a/src/lib/dhcp/option_data_types.cc
+++ b/src/lib/dhcp/option_data_types.cc
@@ -16,6 +16,21 @@
 
 using namespace isc::asiolink;
 
+namespace {
+/// @brief Mast used to compute PSID value
+///
+/// The mask represents the useful bits of the PSID. The value 0 is not used
+/// because the RFC explicitly specifies that PSID value should be ignored if
+/// psid_len is 0. The last entry corresponding to psid_len 16 (which translates
+/// to 'all bits are useful') is added so that an extra check can be omitted.
+std::vector<uint16_t> mask = { 0x0,
+    0x8000, 0xc000, 0xe000, 0xf000,
+    0xf800, 0xfc00, 0xfe00, 0xff00,
+    0xff80, 0xffc0, 0xffe0, 0xfff0,
+    0xfff8, 0xfffc, 0xfffe, 0xffff
+};
+}
+
 namespace isc {
 namespace dhcp {
 
@@ -525,15 +540,7 @@ OptionDataTypeUtil::readPsid(const std::vector<uint8_t>& buf) {
     // psid_len from the left must be set to 0.
     // The value 0 is a special case because the RFC explicitly says that the
     // PSID value should be ignored if psid_len is 0.
-    // The value 16 is a special case for the following check because there is
-    // no bit left that is not used (all bits are useful) and the check would
-    // always be done against ~(0xFFFF) which is 0 (the check is always true).
-    static std::vector<uint16_t> mask = { 0x8000, 0xc000, 0xe000,
-                                  0xf000, 0xf800, 0xfc00, 0xfe00,
-                                  0xff00, 0xff80, 0xffc0, 0xffe0,
-                                  0xfff0, 0xfff8, 0xfffc, 0xfffe };
-    if ((psid_len > 0) && (psid_len < (sizeof(uint16_t) * 8)) &&
-        ((psid & static_cast<uint16_t>(~mask[psid_len - 1])) != 0)) {
+    if ((psid_len > 0) && ((psid & ~mask[psid_len]) != 0)) {
         isc_throw(BadDataTypeCast, "invalid PSID value " << psid
                   << " for a specified PSID length "
                   << static_cast<unsigned>(psid_len));