Project

General

Profile

Actions

Bug #2445

closed

name::Component::compare return value is not guaranteed

Added by Junxiao Shi almost 10 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Low
Assignee:
Category:
Base
Target version:
Start date:
01/28/2015
Due date:
% Done:

100%

Estimated time:

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.

Actions #1

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?

Actions #2

Updated by Junxiao Shi over 9 years ago

note-1 is correct.

Actions #3

Updated by Joao Pereira over 9 years ago

  • Status changed from New to In Progress
Actions #4

Updated by Joao Pereira over 9 years ago

  • Assignee set to Joao Pereira
Actions #5

Updated by Joao Pereira over 9 years ago

  • Status changed from In Progress to Code review

Corrected the issue in the commit:

http://gerrit.named-data.net/2087

Actions #6

Updated by Joao Pereira over 9 years ago

  • % Done changed from 0 to 100
Actions #7

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.

Actions #8

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.

Actions #9

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.

Actions #10

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;.

Actions #11

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

Actions #12

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.

Actions #13

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.

Actions #14

Updated by Junxiao Shi over 9 years ago

  • Description updated (diff)
  • Status changed from Feedback to Closed
Actions

Also available in: Atom PDF