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 almost 10 years ago
The intended is to convert the return to a if statement converting the result into -1, 0, 1?
Updated by Joao Pereira almost 10 years ago
- Status changed from New to In Progress
Updated by Joao Pereira almost 10 years ago
- Status changed from In Progress to Code review
Corrected the issue in the commit:
Updated by Alex Afanasyev almost 10 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 almost 10 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 almost 10 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 almost 10 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 almost 10 years ago
I prefer updating all specs, including Name::compare and anything else that explicitly defines -1
, 0
, and +1
Updated by Junxiao Shi almost 10 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 almost 10 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 almost 10 years ago
- Description updated (diff)
- Status changed from Feedback to Closed