Project

General

Profile

Feature #3932

Add operator== for SigningInfo

Added by Nicholas Gordon over 3 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Category:
Security
Target version:
Start date:
01/25/2017
Due date:
% Done:

100%

Estimated time:

Description

SigningInfo needs to be comparable, so operator== and operator!= need to be defined for them.


Related issues

Blocks NFD - Feature #3818: Readvertise end-host routes into NLSRClosedNicholas Gordon

Actions
#1

Updated by Nicholas Gordon over 3 years ago

  • Blocks Feature #3818: Readvertise end-host routes into NLSR added
#2

Updated by Nicholas Gordon over 3 years ago

  • Status changed from In Progress to Code review
#3

Updated by Junxiao Shi over 3 years ago

  • Tracker changed from Task to Feature
  • Category set to Security

https://gerrit.named-data.net/3624 merges SIGNER_TYPE_PIB_ID into SIGNER_TYPE_ID and SIGNER_TYPE_PIB_KEY into SIGNER_TYPE_KEY.
@Nick should rebase https://gerrit.named-data.net/3622 on the above Change to take advantage of the simplified definitions.

#4

Updated by Nicholas Gordon over 3 years ago

I was having some issues extending the operator== to the other SignatureTypes, but then I wound up here:

bool
SigningInfo::operator==(const SigningInfo& rhs) const
{
  return false;
}

BOOST_AUTO_TEST_CASE(OperatorEqualsDifferentTypes)
{
  SigningInfo info1("key:/my-id/ksk-1");
  SigningInfo info2("key:/my-id/ksk-1");
  // Check equality for key type
  BOOST_CHECK_EQUAL(info1, info2);
  BOOST_CHECK_NE(info1, info2);
  BOOST_CHECK(info1 == info2);
  BOOST_CHECK(info1 != info2);
}

And then:

ndn-cxx git:(master) ✗ build/unit-tests -t Security/TestSigningInfo/OperatorEqualsDifferentTypes
Running 1 test case...
../tests/unit-tests/security/signing-info.t.cpp(210): error in "OperatorEqualsDifferentTypes": check info1 != info2 failed [key:/my-id/ksk-1 == key:/my-id/ksk-1]
../tests/unit-tests/security/signing-info.t.cpp(212): error in "OperatorEqualsDifferentTypes": check info1 != info2 failed

*** 2 failures detected (7 failures expected) in test suite "ndn-cxx Tests"

This is backwards. Outside of this particularly spurious error is that the other test case does not fail (the one currently on gerrit). Additionally, I had problems where std::cout statements would not print in the operator== definition, and

Clearly something is wrong. However, I have tried distcleans, using clang and gcc, as much as I could think of. However, something is broken. Should I update my compilers and linker?

#5

Updated by Junxiao Shi over 3 years ago

BOOST_CHECK_EQUAL(info1, info2);
BOOST_CHECK_NE(info1, info2);
BOOST_CHECK(info1 == info2);
BOOST_CHECK(info1 != info2);

The first and third assertions expect info1 to be equal to info2. The second and fourth assertions expect info1 to be unequal to info2. This cannot possibly work: if equality comparison operators are implemented correctly, info1 cannot be both equal and unequal to info2.

#6

Updated by Nicholas Gordon over 3 years ago

Indeed, you are right. What I posted was for the point of illustration. Doing that showed that the operator seems to have no effect on the unit test. In fact, I experimented with making operator== simply return false, then later true, but the outcome of the unit tests did not change. Further, the other unit test succeeds as expected, even though the tests could not possibly pass if operator== blindly returns a value. I have posted the code on the gerrit issue so others can examine it.

#7

Updated by Nicholas Gordon over 3 years ago

I have solved the problem. I prefer to use clang as my main compiler because it is often faster and the error messages are clearer. However, Davide notes https://redmine.named-data.net/issues/3299 that clang-3.8 cannot be used to compile ndn-cxx due to an ABI issue. This was the problem exactly for me. Upon reading this I recompiled and installed ndn-cxx with GCC, and the problem was gone.

#8

Updated by Junxiao Shi over 3 years ago

  • Status changed from Code review to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF