BIND 10 #1774: use uint8_t for "characters" of Name data
BIND 10 Development
do-not-reply at isc.org
Fri Jun 22 21:30:13 UTC 2012
#1774: use uint8_t for "characters" of Name data
-------------------------------------+-------------------------------------
Reporter: | Owner: jinmei
jinmei | Status: reviewing
Type: | Milestone:
defect | Sprint-20120703
Priority: | Resolution:
medium | Sensitive: 0
Component: | Sub-Project: DNS
libdns++ | Estimated Difficulty: 3
Keywords: | Total Hours: 0
Defect Severity: N/A |
Feature Depending on Ticket: |
Add Hours to Ticket: 0 |
Internal?: 0 |
-------------------------------------+-------------------------------------
Comment (by jinmei):
- maybe a matter of taste, especially because it's class-private data,
but I'd personally define a custom type instead of directly using
`string<uint8_t>`:
{{{#!cpp
typedef std::basic_string<uint8_t> NameString;
NameString ndata_;
}}}
- labelsequence should use uint8_t rather than unsigned char
- same for messagenrenderer.
- it looks like some other places in name.cc could use uint8_t instead
of unsigned char
- this doesn't seem to be correct in various points:
{{{#!cpp
return (std::strncmp((const char*) data,
(const char*) other_data,
len) == 0);
}}}
First, although this is not because of this branch, I suspect
str[n]cmp cannot be used here because data/other_data can contain \0
as part of a valid label. I also suggest adding a specific test
that can either confirm or deny this. And, in any case, memcmp is a
better choice here, and probably the only correct choice if my
suspicion is correct. Third, we shouldn't deceive compiler by cast
in this type of case. Fourth, even if this were one of such rare
cases, we should never use C-style cast. Combining these, this part
should be:
{{{#!cpp
// and type of data/other_data should be changed to uint8_t
return (std::memcmp(data, other_data, len) == 0);
}}}
- same sense of comment applies to labelsequence test. I'd first
clarify the type of getData() and test data, too. On top of that
I'd make overall cleanup. If it still seems to require casting from
unsigned char* to char*, something should be wrong.
--
Ticket URL: <http://bind10.isc.org/ticket/1774#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list