openssl: fix equality test methods of OpenSSL::BN

* ext/openssl/ossl_bn.c (try_convert_to_bnptr): Extracted from
  GetBNPtr(). This doesn't raise exception but returns NULL on error.
  (GetBNPtr): Raise TypeError if conversion fails.
  (ossl_bn_eq): Implement BN#==.
  (ossl_bn_eql): #eql? should not raise TypeError even if the argument
  is not compatible with BN.
  (ossl_bn_hash): Implement BN#hash.

* ext/openssl/ossl_bn.c (Init_ossl_bn): Define #== and #hash.

* test/openssl/test_bn.rb: Test BN#eql?, #== and #hash

3 files changed, 103 insertions, 17 deletions

diff --git a/ChangeLog b/ChangeLog
index c3ffe672df..25d6d25176 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+Wed May 18 11:19:59 2016  Kazuki Yamaguchi  <k@rhe.jp>
+
+	* ext/openssl/ossl_bn.c (try_convert_to_bnptr): Extracted from
+	  GetBNPtr(). This doesn't raise exception but returns NULL on error.
+	  (GetBNPtr): Raise TypeError if conversion fails.
+	  (ossl_bn_eq): Implement BN#==.
+	  (ossl_bn_eql): #eql? should not raise TypeError even if the argument
+	  is not compatible with BN.
+	  (ossl_bn_hash): Implement BN#hash.
+
+	* ext/openssl/ossl_bn.c (Init_ossl_bn): Define #== and #hash.
+
+	* test/openssl/test_bn.rb: Test BN#eql?, #== and #hash
+
 Wed May 18 10:17:41 2016  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* include/ruby/ruby.h (RB_INTEGER_TYPE_P): new macro and
diff --git a/ext/openssl/ossl_bn.c b/ext/openssl/ossl_bn.c
index fba86cdd44..0af6c68466 100644
--- a/ext/openssl/ossl_bn.c
+++ b/ext/openssl/ossl_bn.c
@@ -82,8 +82,8 @@ ossl_bn_new(const BIGNUM *bn)
     return obj;
 }
 
-BIGNUM *
-GetBNPtr(VALUE obj)
+static BIGNUM *
+try_convert_to_bnptr(VALUE obj)
 {
     BIGNUM *bn = NULL;
     VALUE newobj;
@@ -100,14 +100,20 @@ GetBNPtr(VALUE obj)
 	}
 	SetBN(newobj, bn); /* Handle potencial mem leaks */
 	break;
-    case T_NIL:
-	break;
-    default:
-	ossl_raise(rb_eTypeError, "Cannot convert into OpenSSL::BN");
     }
     return bn;
 }
 
+BIGNUM *
+GetBNPtr(VALUE obj)
+{
+    BIGNUM *bn = try_convert_to_bnptr(obj);
+    if (!bn)
+	ossl_raise(rb_eTypeError, "Cannot convert into OpenSSL::BN");
+
+    return bn;
+}
+
 /*
  * Private
  */
@@ -841,21 +847,78 @@ BIGNUM_CMP(ucmp)
 
 /*
  *  call-seq:
- *     big.eql?(obj) => true or false
+ *     bn == obj => true or false
  *
- *  Returns <code>true</code> only if <i>obj</i> is a
- *  <code>Bignum</code> with the same value as <i>big</i>. Contrast this
+ *  Returns +true+ only if +obj+ has the same value as +bn+. Contrast this
+ *  with OpenSSL::BN#eql?, which requires obj to be OpenSSL::BN.
  */
 static VALUE
-ossl_bn_eql(VALUE self, VALUE other)
+ossl_bn_eq(VALUE self, VALUE other)
 {
-    if (ossl_bn_cmp(self, other) == INT2FIX(0)) {
+    BIGNUM *bn1, *bn2;
+
+    GetBN(self, bn1);
+    /* BNPtr may raise, so we can't use here */
+    bn2 = try_convert_to_bnptr(other);
+
+    if (bn2 && !BN_cmp(bn1, bn2)) {
 	return Qtrue;
     }
     return Qfalse;
 }
 
 /*
+ *  call-seq:
+ *     bn.eql?(obj) => true or false
+ *
+ *  Returns <code>true</code> only if <i>obj</i> is a
+ *  <code>OpenSSL::BN</code> with the same value as <i>big</i>. Contrast this
+ *  with OpenSSL::BN#==, which performs type conversions.
+ */
+static VALUE
+ossl_bn_eql(VALUE self, VALUE other)
+{
+    BIGNUM *bn1, *bn2;
+
+    if (!rb_obj_is_kind_of(other, cBN))
+	return Qfalse;
+    GetBN(self, bn1);
+    GetBN(other, bn2);
+
+    return BN_cmp(bn1, bn2) ? Qfalse : Qtrue;
+}
+
+/*
+ *  call-seq:
+ *     bn.hash => Integer
+ *
+ *  Returns a hash code for this object.
+ *
+ *  See also Object#hash.
+ */
+static VALUE
+ossl_bn_hash(VALUE self)
+{
+    BIGNUM *bn;
+    VALUE hash;
+    unsigned char *buf;
+    int len;
+
+    GetBN(self, bn);
+    len = BN_num_bytes(bn);
+    buf = xmalloc(len);
+    if (BN_bn2bin(bn, buf) != len) {
+	xfree(buf);
+	ossl_raise(eBNError, NULL);
+    }
+
+    hash = INT2FIX(rb_memhash(buf, len));
+    xfree(buf);
+
+    return hash;
+}
+
+/*
  * call-seq:
  *    bn.prime? => true | false
  *    bn.prime?(checks) => true | false
@@ -982,8 +1045,9 @@ Init_ossl_bn(void)
     rb_define_alias(cBN, "<=>", "cmp");
     rb_define_method(cBN, "ucmp", ossl_bn_ucmp, 1);
     rb_define_method(cBN, "eql?", ossl_bn_eql, 1);
-    rb_define_alias(cBN, "==", "eql?");
-    rb_define_alias(cBN, "===", "eql?");
+    rb_define_method(cBN, "hash", ossl_bn_hash, 0);
+    rb_define_method(cBN, "==", ossl_bn_eq, 1);
+    rb_define_alias(cBN, "===", "==");
     rb_define_method(cBN, "zero?", ossl_bn_is_zero, 0);
     rb_define_method(cBN, "one?", ossl_bn_is_one, 0);
     /* is_word */
diff --git a/test/openssl/test_bn.rb b/test/openssl/test_bn.rb
index 848d1feb43..415bd74c79 100644
--- a/test/openssl/test_bn.rb
+++ b/test/openssl/test_bn.rb
@@ -43,10 +43,18 @@ class OpenSSL::TestBN < Test::Unit::TestCase
     assert_equal(true, OpenSSL::BN.new((2 ** 127 - 1).to_s(16), 16).prime?(1))
   end
 
-  def test_cmp_nil
-    bn = OpenSSL::BN.new('1')
-    assert_equal(false, bn == nil)
-    assert_equal(true,  bn != nil)
+  def test_cmp
+    bn1 = OpenSSL::BN.new('1')
+    bn2 = OpenSSL::BN.new('1')
+    bn3 = OpenSSL::BN.new('2')
+    assert_equal(false, bn1 == nil)
+    assert_equal(true,  bn1 != nil)
+    assert_equal(true, bn1 == bn2)
+    assert_equal(false, bn1 == bn3)
+    assert_equal(true, bn1.eql?(bn2))
+    assert_equal(false, bn1.eql?(bn3))
+    assert_equal(bn1.hash, bn2.hash)
+    assert_not_equal(bn3.hash, bn1.hash)
   end
 end