BIND 10 #1883: define tp_hash for some basic isc.dns classes
BIND 10 Development
do-not-reply at isc.org
Thu Jul 5 21:34:52 UTC 2012
#1883: define tp_hash for some basic isc.dns classes
-------------------------------------+-------------------------------------
Reporter: | Owner: jinmei
jinmei | Status: reviewing
Type: task | Milestone:
Priority: high | Sprint-20120717
Component: | Resolution:
libdns++ | Sensitive: 0
Keywords: | Sub-Project: DNS
Defect Severity: N/A | Estimated Difficulty: 5
Feature Depending on Ticket: | Total Hours: 0
Add Hours to Ticket: 0 |
Internal?: 0 |
-------------------------------------+-------------------------------------
Comment (by jinmei):
Replying to [comment:5 jelte]:
> I had a comment about Py_hash_t but I see we already have #2026 for that
(do we care about the implicit cast there, btw?) :)
Hmm, not sure what exactly you meant by the implicit cast, but on
looking into it more closely, I noticed we actually need to do care
about it. Py_hash_t is a signed integer, and tp_hash() is not
expected to return -1. So we need to ensure two things:
- the conversion to Py_hash_t is safe (not result in an undefined
behavior)
- the result is never -1
In the case of the extension to `RRType`, it's unlikely to be a real
issue in practice because the original hash value is of uint16_t, and
it size is quite likely to be smaller than sizeof(Py_hash_t). But it
could be more realistic concern for `Name` because the original hash
is of size_t, and (in my environment) it has the same size as that of
Py_hash_t.
So I introduced a wrapper converter and used it for all existing
cases.
> I have one tiny suggestion for the unit tests; add
> {{{
> self.assertEqual(hash(RRType.AAAA()), hash(RRType("Type28")))
> }}}
> as well (or a similar one).
Okay, added.
--
Ticket URL: <http://bind10.isc.org/ticket/1883#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list