Discussion:
rb_tree_iterate(3) documentation vs. implementation
David Young
2012-08-28 16:59:02 UTC
Permalink
Regarding pr/46034, the manual describes one behavior for
rb_tree_iterate(3), while the implementation actually provides another.
The behavior described in the manual seems much more sensible and
useful, and I would like to change the implementation to match: see the
attached patch.

Dave
--
David Young
***@pobox.com Urbana, IL (217) 721-9981
David Young
2012-08-28 17:39:17 UTC
Permalink
Post by David Young
Regarding pr/46034, the manual describes one behavior for
rb_tree_iterate(3), while the implementation actually provides another.
The behavior described in the manual seems much more sensible and
useful, and I would like to change the implementation to match: see the
attached patch.
Joerg points out that my patch isn't complete, the RB_TREE_MIN() and
_MAX() routines need to be fixed.

Dave
--
David Young
***@pobox.com Urbana, IL (217) 721-9981
Mindaugas Rasiukevicius
2012-08-28 17:57:05 UTC
Permalink
Post by David Young
Regarding pr/46034, the manual describes one behavior for
rb_tree_iterate(3), while the implementation actually provides another.
The behavior described in the manual seems much more sensible and
useful, and I would like to change the implementation to match: see the
attached patch.
- The patch is wrong as is: all callers of rb_tree_iterate(3), including
RB_{MIN,MAX}, but there are quite more in the tree need to be audited
and where required fix.

- There is also PR/45893. The reason why these changes were not made are
concerns about breaking backwards compatibility (apparently, there are
3rd party users of this library already). In theory, it is not too late,
as netbsd-6 will be the first release shipping rbtree(3), but we need to
reach the consensus on this.
--
Mindaugas
Paul Goyette
2012-08-28 18:12:59 UTC
Permalink
Post by Mindaugas Rasiukevicius
- There is also PR/45893. The reason why these changes were not made are
concerns about breaking backwards compatibility (apparently, there are
3rd party users of this library already). In theory, it is not too late,
as netbsd-6 will be the first release shipping rbtree(3), but we need to
reach the consensus on this.
Seems to me, we ought to get this "right" before we formally ship. The
"early adopters" who are already using rbtree(3) already should be few
in number and hopefully we could work with them to adapt to the changes.


-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------
Jeff Rizzo
2012-08-29 16:11:34 UTC
Permalink
Post by Paul Goyette
Post by Mindaugas Rasiukevicius
- There is also PR/45893. The reason why these changes were not made are
concerns about breaking backwards compatibility (apparently, there are
3rd party users of this library already). In theory, it is not too late,
as netbsd-6 will be the first release shipping rbtree(3), but we need to
reach the consensus on this.
Seems to me, we ought to get this "right" before we formally ship.
The "early adopters" who are already using rbtree(3) already should be
few in number and hopefully we could work with them to adapt to the
changes.
Why is it that people still think that at this late date, we would
entertain changing an API in NetBSD-6 when a release candidate is built
and about to be announced?!?

+j
Thor Lancelot Simon
2012-08-29 16:31:42 UTC
Permalink
Post by Jeff Rizzo
Post by Paul Goyette
Post by Mindaugas Rasiukevicius
- There is also PR/45893. The reason why these changes were not made are
concerns about breaking backwards compatibility (apparently, there are
3rd party users of this library already). In theory, it is not too late,
as netbsd-6 will be the first release shipping rbtree(3), but we need to
reach the consensus on this.
Seems to me, we ought to get this "right" before we formally ship.
The "early adopters" who are already using rbtree(3) already
should be few in number and hopefully we could work with them to
adapt to the changes.
Why is it that people still think that at this late date, we would
entertain changing an API in NetBSD-6 when a release candidate is
built and about to be announced?!?
Because this is a very serious API bug, and it might be preferable to
never ship a version of NetBSD that has it, rather than have to support
two APIs (broken and non-broken) forever?

Thor
Thor Lancelot Simon
2012-08-29 17:06:52 UTC
Permalink
Post by Thor Lancelot Simon
Post by Jeff Rizzo
Post by Paul Goyette
Post by Mindaugas Rasiukevicius
- There is also PR/45893. The reason why these changes were not made are
concerns about breaking backwards compatibility (apparently, there are
3rd party users of this library already). In theory, it is not too late,
as netbsd-6 will be the first release shipping rbtree(3), but we need to
reach the consensus on this.
Seems to me, we ought to get this "right" before we formally ship.
The "early adopters" who are already using rbtree(3) already
should be few in number and hopefully we could work with them to
adapt to the changes.
Why is it that people still think that at this late date, we would
entertain changing an API in NetBSD-6 when a release candidate is
built and about to be announced?!?
Because this is a very serious API bug, and it might be preferable to
never ship a version of NetBSD that has it, rather than have to support
two APIs (broken and non-broken) forever?
I will point out that this "very serious API bug" has been known
about since BEFORE the netbsd-6 branch; if it was not worth the
time of those who care about this API to have done something about
it in the months since then, why does it suddenly get to percolate
to the top of priorities now, when it would hold up the release and
impact *my* (and other release engineers') work?
Because someone who did not know about it since BEFORE the netbsd-6
branch pointed it out publically and a larger population of developers
realized it's a significant problem?

You're the release engineer. Go ahead, make a decision. Nobody's saying
it's not up to you: plainly, it is.

But, even though clearly there was a screwup in not bringing this
problem to releng's attention (by checking in a fix and submitting a
pullup request) on the part of the people who originally noticed it, I
would advocate that the burden of ongoing maintenance if this is left
how it is might well outweigh the delay it causes to the release, if
it is changed at this late date.

Thor
Jeff Rizzo
2012-08-29 16:48:45 UTC
Permalink
Post by Thor Lancelot Simon
Post by Jeff Rizzo
Post by Paul Goyette
Post by Mindaugas Rasiukevicius
- There is also PR/45893. The reason why these changes were not made are
concerns about breaking backwards compatibility (apparently, there are
3rd party users of this library already). In theory, it is not too late,
as netbsd-6 will be the first release shipping rbtree(3), but we need to
reach the consensus on this.
Seems to me, we ought to get this "right" before we formally ship.
The "early adopters" who are already using rbtree(3) already
should be few in number and hopefully we could work with them to
adapt to the changes.
Why is it that people still think that at this late date, we would
entertain changing an API in NetBSD-6 when a release candidate is
built and about to be announced?!?
Because this is a very serious API bug, and it might be preferable to
never ship a version of NetBSD that has it, rather than have to support
two APIs (broken and non-broken) forever?
I will point out that this "very serious API bug" has been known about
since BEFORE the netbsd-6 branch; if it was not worth the time of those
who care about this API to have done something about it in the months
since then, why does it suddenly get to percolate to the top of
priorities now, when it would hold up the release and impact *my* (and
other release engineers') work?

There are very few people in the NetBSD community who don't complain
about how long it is between releases; this sort of thing is EXACTLY
what delays releases and discourages release engineers. People have to
take responsibility for acting in a timely fashion. If it's an important
issue, and you've noticed it - push it to resolution NOW, and please
don't wait until it's screwing up other people's stuff.

As you can tell, I'm very frustrated, and feel like a large segment of
the developer population does not respect releng's time and effort. I'm
sorry if this rant feels personally pointed at any one person, I'm
certainly not intending it that way. However, any release is going to
have bugs - some of them are likely to be the sorts of things we have to
deal with for years. There comes a point where you just have to say,
"I'm sorry, it's too late". If consensus can be reached, we can fix the
documentation for the release to deprecate the current state of things,
and give users of the library a heads up that we feel there is a design
flaw that we intend to address - but changing it *now*? Madness.

+j
David Young
2012-08-29 18:11:32 UTC
Permalink
Post by Jeff Rizzo
Post by Paul Goyette
Post by Mindaugas Rasiukevicius
- There is also PR/45893. The reason why these changes were not made are
concerns about breaking backwards compatibility (apparently, there are
3rd party users of this library already). In theory, it is not too late,
as netbsd-6 will be the first release shipping rbtree(3), but we need to
reach the consensus on this.
Seems to me, we ought to get this "right" before we formally ship.
The "early adopters" who are already using rbtree(3) already
should be few in number and hopefully we could work with them to
adapt to the changes.
Why is it that people still think that at this late date, we would
entertain changing an API in NetBSD-6 when a release candidate is
built and about to be announced?!?
I am opposed to delaying the release for this, myself.

In a subsequent release we can rename all of the rb_tree_*() routines
to rbtree_*(). The bad old rb_tree_iterate() can be a wrapper for
rbtree_iterate() that swaps RB_DIR_LEFT for RB_DIR_RIGHT (and vice
versa) when the 2nd argument is NULL. Every other rb_tree_*() can be an
alias for the corresponding rbtree_*(). Or something like that.

Dave
--
David Young
***@pobox.com Urbana, IL (217) 721-9981
Mindaugas Rasiukevicius
2014-03-31 16:07:12 UTC
Permalink
Post by David Young
Regarding pr/46034, the manual describes one behavior for
rb_tree_iterate(3), while the implementation actually provides another.
The behavior described in the manual seems much more sensible and
useful, and I would like to change the implementation to match: see the
attached patch.
This is almost 2-years old thread, but FYI:

https://developer.apple.com/library/mac/documentation/Darwin/Reference/Manpages/man3/rbtree.3.html

See "IMPLEMENTATION DETAILS" section. Apparently, Apple decided to change
the API i.e. the behaviour of rb_tree_iterate(3). So, we are incompatible.

I guess the lesson is: if vendors do not communicate with us, we should
just go ahead with our changes instead of worrying about them.
--
Mindaugas
Loading...