Bug #2445
closedname::Component::compare return value is not guaranteed
100%
Description
name::Component::compare
documentation defines its return value to be either -1, 0, or 1.
Its implementation contains return std::memcmp(...
.
std::memcmp
returns either "negative value", 0, or "positive value", where "negative value" isn't guaranteed to be -1, and "positive value" isn't guaranteed to be 1.
The result would be compiler dependent.
Updated by Joao Pereira over 9 years ago
The intended is to convert the return to a if statement converting the result into -1, 0, 1?
Updated by Joao Pereira over 9 years ago
- Status changed from In Progress to Code review
Corrected the issue in the commit:
Updated by Alex Afanasyev over 9 years ago
What about alternative solution: change documentation. I'm not aware of anybody actually comparing to +/- 1 and just any negative, 0, or any positive value is perfectly fine result for name::Component::compare.
Updated by Junxiao Shi over 9 years ago
change documentation
The documented behavior can be implemented correctly and efficiently, so there's no reason to loosen requirements.
Updated by Alex Afanasyev over 9 years ago
I disagree. It was an error to define behavior that narrowly in the documentation. An equivalent for this operation (and what this method was modeled against) is std::string::compare
that defines the return value as <0
, 0
, or >0
.
We should fix documentation error, not enforce unnecessarily defined strictness in the docs.
Updated by Junxiao Shi over 9 years ago
If we decide to loosen the spec to negative/zero/positive, same should apply to Name::compare
, and the last return
in Name::compare
impl could be changed to return count1 - count2;
.
Updated by Alex Afanasyev over 9 years ago
I prefer updating all specs, including Name::compare and anything else that explicitly defines -1
, 0
, and +1
Updated by Junxiao Shi over 9 years ago
- Status changed from Code review to Feedback
- Target version set to v0.4
@Joao, please rework according to note-10 and note-11.
Updated by Joao Pereira over 9 years ago
Here is the link with the changes suggested.
http://gerrit.named-data.net/2087
See if the documentation looks correct now.
Updated by Junxiao Shi over 9 years ago
- Description updated (diff)
- Status changed from Feedback to Closed