Re: additional queue macro
On Thu, 4 Jul 2002, Julian Elischer wrote: > > On Thu, 4 Jul 2002, Daniel Eischen wrote: > > > On Thu, 4 Jul 2002, Julian Elischer wrote: > > > > > 2/ > > > We could add a new macro/method that is slightly less efficient than the > > > current FOREACH macros, but allows element removal. > > > Exisiting methods would no change. > > > > As wollman pointed out, we already assume that it is safe to > > remove elements using the existing macros. Adding another > > interface to do the same thing kinda imples that existing > > behaviour may change. As proposed though, the new macros > > would not only allow removals, but also modification of > > the removed element while still walking the list. These might > > be useful. > > > > In this rare case Garrett was wrong. Only He assumed that. I assumed that also, and libc_r currently assumes that. Perhaps I am in the minority, but maybe there are more reliances on this than you think? Anyways, I don't have a problem transitioning to another set of macros when they are provided. -- Dan Eischen To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Thu, 4 Jul 2002, Daniel Eischen wrote:
> On Thu, 4 Jul 2002, Julian Elischer wrote:
>
> > there are two proposals floatingat the moment..
> >
> > 1/ I added debugging stuff to TAILQ to help find bad usages in KSE.
> > Qusetion/proposal: Should I extend this to other types and add it to the
> > file (or not delete what is there now)
>
> I was suggesting that you add macros for debugging purposes instead
> of potentially changing existing behaviour. The way you've got it
> now is OK I guess, just as long as it somehow doesn't get enabled
> or changed in userland. Perhaps it would even break consumers
> of it in the kernel, though, too.
>
> > 2/
> > We could add a new macro/method that is slightly less efficient than the
> > current FOREACH macros, but allows element removal.
> > Exisiting methods would no change.
>
> As wollman pointed out, we already assume that it is safe to
> remove elements using the existing macros. Adding another
> interface to do the same thing kinda imples that existing
> behaviour may change. As proposed though, the new macros
> would not only allow removals, but also modification of
> the removed element while still walking the list. These might
> be useful.
>
In this rare case Garrett was wrong. Only He assumed that. Most people do
not assume that, in fact 'folk lore' in general is that it is NOT safe to
do that. In fact it is NOT safe to do so if you are going to do anythign
WITH that element while still in the loop. The kernel does not do it
anywhere I could find, and the man pages specifically avoid doing that.
There is no historical precedent because the iterators are already a
recent FreeBSD (phk) addition. CSRG did not have them.
The ability do do:
TAILQ_FOREACH_REMOVABLE(thread, runqueue...) {
if (thread should be suspended) {
TAILQ_REMOVE(thread);
TAILQ_INSERT_TAIL(thread, idlequeue...);
}
}
would be very nice.
> --
> Dan Eischen
>
>
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Thu, 4 Jul 2002, Julian Elischer wrote: > there are two proposals floatingat the moment.. > > 1/ I added debugging stuff to TAILQ to help find bad usages in KSE. > Qusetion/proposal: Should I extend this to other types and add it to the > file (or not delete what is there now) I was suggesting that you add macros for debugging purposes instead of potentially changing existing behaviour. The way you've got it now is OK I guess, just as long as it somehow doesn't get enabled or changed in userland. Perhaps it would even break consumers of it in the kernel, though, too. > 2/ > We could add a new macro/method that is slightly less efficient than the > current FOREACH macros, but allows element removal. > Exisiting methods would no change. As wollman pointed out, we already assume that it is safe to remove elements using the existing macros. Adding another interface to do the same thing kinda imples that existing behaviour may change. As proposed though, the new macros would not only allow removals, but also modification of the removed element while still walking the list. These might be useful. -- Dan Eischen To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
there are two proposals floatingat the moment.. 1/ I added debugging stuff to TAILQ to help find bad usages in KSE. Qusetion/proposal: Should I extend this to other types and add it to the file (or not delete what is there now) 2/ We could add a new macro/method that is slightly less efficient than the current FOREACH macros, but allows element removal. Exisiting methods would no change. On Thu, 4 Jul 2002, Daniel Eischen wrote: > On Thu, 4 Jul 2002, Julian Elischer wrote: > > that was teh plan... we're just discussing the name.. > > TAILQ_FOREACH_SAFE ? > > Oh, I thought the initial proposal was to add a _new_ interface > that allowed safe removals while traversing the list (and allow > the existing macros to be changed for debugging purposes/extra > sanity checks). > > -- > Dan Eischen > > To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Thu, 4 Jul 2002, Julian Elischer wrote: > that was teh plan... we're just discussing the name.. > TAILQ_FOREACH_SAFE ? Oh, I thought the initial proposal was to add a _new_ interface that allowed safe removals while traversing the list (and allow the existing macros to be changed for debugging purposes/extra sanity checks). -- Dan Eischen To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
that was teh plan... we're just discussing the name.. TAILQ_FOREACH_SAFE ? On Thu, 4 Jul 2002, Daniel Eischen wrote: > On Wed, 3 Jul 2002, Julian Elischer wrote: > > On Wed, 3 Jul 2002, Neal Fachan wrote: > > > > > We've got local changes (which I've attached) where the name is > > > *_FOREACH_REMOVE. We didn't add reverse removable iterators. Also, the > > > temp variable is the second argument. I can't think of a way of doing it > > > without having the externally declare the temporary variable. > > > > > A I like it and you've even done thge man page.. > > > > *_FOREACH_REMOVE however suggests that it is going to try remove > > something.. > > Instead of potentially changing the existing *_FOREACH behaviour, > why not just add *_FOREACH_CHECKED or *_FOREACH_PEDANTIC that > adds the desired behaviour. Or *_FOREACH_DEBUG... > > -- > Dan > > > To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Wed, 3 Jul 2002, Julian Elischer wrote: > On Wed, 3 Jul 2002, Neal Fachan wrote: > > > We've got local changes (which I've attached) where the name is > > *_FOREACH_REMOVE. We didn't add reverse removable iterators. Also, the > > temp variable is the second argument. I can't think of a way of doing it > > without having the externally declare the temporary variable. > > > A I like it and you've even done thge man page.. > > *_FOREACH_REMOVE however suggests that it is going to try remove > something.. Instead of potentially changing the existing *_FOREACH behaviour, why not just add *_FOREACH_CHECKED or *_FOREACH_PEDANTIC that adds the desired behaviour. Or *_FOREACH_DEBUG... -- Dan To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
We've got local changes (which I've attached) where the name is
*_FOREACH_REMOVE. We didn't add reverse removable iterators. Also, the
temp variable is the second argument. I can't think of a way of doing it
without having the externally declare the temporary variable.
On Tue, Jul 02, 2002 at 04:54:43PM -0700, Julian Elischer wrote:
>
>
> On Tue, 2 Jul 2002, Julian Elischer wrote:
>
> Having just re-read my own mail
>
> I think I agree with jonathan now..
> I think we neeed to either:
> 1/ augment the man page giving an example of how to do
> multiple removes from a list/queue.
> 2/ Explain in detail why using _FOREACH()
> is bad for this, and showing the alternative.
> 3/ Add something to the API that makes this easy to do.
> designing the API addition is tricky. Jonathan's effort
> was quite good, though I wonder if there is any way we can get it done
> without needing the decalration of 'tmp' separatly.
> (I can't think of a way).
>
>
>
> > /*
> > * Move any threads that should be suspended from the run queue
> > * to the suspend queue.
> > */
> > TAILQ_FOREACH(from run queue) {
> > if (something) {
> > TAILQ_REMOVE(element from run queue)
> > TAILQ_INSERT_TAIL(onto suspend queue)
> > }
> > }
> >
> > Now, at first glance, the documentation suggests this should work, even
> > though we know it won't. but it doesn't crash.. it just terminates on the
> > first transfer because it reaches the end of the queue.. the suspend queue
> > that is..
>
> TAILQ_FOREACH_REMOVABLE or TAILQ_FOREACH_SAFE
> (I prefer the first) are my suggestions for the name.)
>
>
>
>
> To Unsubscribe: send mail to [EMAIL PROTECTED]
> with "unsubscribe freebsd-current" in the body of the message
--
Neal Fachan
[EMAIL PROTECTED]
Index: share/man/man3/queue.3
===
RCS file: /usr/local/ncvs/atera/src/share/man/man3/queue.3,v
retrieving revision 1.1
diff -u -p -r1.1 queue.3
--- share/man/man3/queue.3 9 Mar 2002 02:34:15 - 1.1
+++ share/man/man3/queue.3 3 Jul 2002 20:46:19 -
@@ -1,4 +1,4 @@
-.\" Copyright (c) 1993
+tail queue Copyright (c) 1993
.\"The Regents of the University of California. All rights reserved.
.\"
.\" Redistribution and use in source and binary forms, with or without
@@ -40,6 +40,7 @@
.Nm SLIST_ENTRY ,
.Nm SLIST_FIRST ,
.Nm SLIST_FOREACH ,
+.Nm SLIST_FOREACH_REMOVE ,
.Nm SLIST_HEAD ,
.Nm SLIST_HEAD_INITIALIZER ,
.Nm SLIST_INIT ,
@@ -52,6 +53,7 @@
.Nm STAILQ_ENTRY ,
.Nm STAILQ_FIRST ,
.Nm STAILQ_FOREACH ,
+.Nm STAILQ_FOREACH_REMOVE ,
.Nm STAILQ_HEAD ,
.Nm STAILQ_HEAD_INITIALIZER ,
.Nm STAILQ_INIT ,
@@ -66,6 +68,7 @@
.Nm LIST_ENTRY ,
.Nm LIST_FIRST ,
.Nm LIST_FOREACH ,
+.Nm LIST_FOREACH_REMOVE ,
.Nm LIST_HEAD ,
.Nm LIST_HEAD_INITIALIZER ,
.Nm LIST_INIT ,
@@ -78,6 +81,7 @@
.Nm TAILQ_ENTRY ,
.Nm TAILQ_FIRST ,
.Nm TAILQ_FOREACH ,
+.Nm TAILQ_FOREACH_REMOVE ,
.Nm TAILQ_FOREACH_REVERSE ,
.Nm TAILQ_HEAD ,
.Nm TAILQ_HEAD_INITIALIZER ,
@@ -99,6 +103,7 @@ lists and tail queues
.Fn SLIST_ENTRY "TYPE"
.Fn SLIST_FIRST "SLIST_HEAD *head"
.Fn SLIST_FOREACH "TYPE *var" "SLIST_HEAD *head" "SLIST_ENTRY NAME"
+.Fn SLIST_FOREACH_REMOVE "TYPE *var" "TYPE *tmpvar" "SLIST_HEAD *head" "SLIST_ENTRY
+NAME"
.Fn SLIST_HEAD "HEADNAME" "TYPE"
.Fn SLIST_HEAD_INITIALIZER "SLIST_HEAD head"
.Fn SLIST_INIT "SLIST_HEAD *head"
@@ -112,6 +117,7 @@ lists and tail queues
.Fn STAILQ_ENTRY "TYPE"
.Fn STAILQ_FIRST "STAILQ_HEAD *head"
.Fn STAILQ_FOREACH "TYPE *var" "STAILQ_HEAD *head" "STAILQ_ENTRY NAME"
+.Fn STAILQ_FOREACH_REMOVE "TYPE *var" "TYPE *tmpvar" "STAILQ_HEAD *head"
+"STAILQ_ENTRY NAME"
.Fn STAILQ_HEAD "HEADNAME" "TYPE"
.Fn STAILQ_HEAD_INITIALIZER "STAILQ_HEAD head"
.Fn STAILQ_INIT "STAILQ_HEAD *head"
@@ -127,6 +133,7 @@ lists and tail queues
.Fn LIST_ENTRY "TYPE"
.Fn LIST_FIRST "LIST_HEAD *head"
.Fn LIST_FOREACH "TYPE *var" "LIST_HEAD *head" "LIST_ENTRY NAME"
+.Fn LIST_FOREACH_REMOVE "TYPE *var" "TYPE *tmpvar" "LIST_HEAD *head" "LIST_ENTRY NAME"
.Fn LIST_HEAD "HEADNAME" "TYPE"
.Fn LIST_HEAD_INITIALIZER "LIST_HEAD head"
.Fn LIST_INIT "LIST_HEAD *head"
@@ -140,6 +147,7 @@ lists and tail queues
.Fn TAILQ_ENTRY "TYPE"
.Fn TAILQ_FIRST "TAILQ_HEAD *head"
.Fn TAILQ_FOREACH "TYPE *var" "TAILQ_HEAD *head" "TAILQ_ENTRY NAME"
+.Fn TAILQ_FOREACH_REMOVE "TYPE *var" "TYPE *tmpvar" "TAILQ_HEAD *head" "TAILQ_ENTRY
+NAME"
.Fn TAILQ_FOREACH_REVERSE "TYPE *var" "TAILQ_HEAD *head" "HEADNAME" "TAILQ_ENTRY NAME"
.Fn TAILQ_HEAD "HEADNAME" "TYPE"
.Fn TAILQ_HEAD_INITIALIZER "TAILQ_HEAD head"
@@ -316,6 +324,24 @@ turn to
.Fa var .
.Pp
The macro
+.Nm SLIST_FOREACH_REMOVE
+traverses the list referenced by
+.Fa head
+in the forward direction, assigning each element in
+turn to
+.Fa var .
+Unlike
+.Nm SLIST_FOREACH ,
+.Fa var
+is never dereferenced after the body of the loop, making it legal to remove
+.Fa var
+from the list or to call
+.Nm free
+on
+.Fa var
+in the loop body.
+.P
Re: additional queue macro
I agree that it's unfortunately named. The idea was that it's safe to remove from within the iterator, not that the iterator is doing the removing... On Wed, Jul 03, 2002 at 04:04:53PM -0700, Julian Elischer wrote: > > > On Wed, 3 Jul 2002, Neal Fachan wrote: > > > We've got local changes (which I've attached) where the name is > > *_FOREACH_REMOVE. We didn't add reverse removable iterators. Also, the > > temp variable is the second argument. I can't think of a way of doing it > > without having the externally declare the temporary variable. > > > A I like it and you've even done thge man page.. > > *_FOREACH_REMOVE however suggests that it is going to try remove > something.. > > -- Neal Fachan [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Wed, 3 Jul 2002, Neal Fachan wrote: > We've got local changes (which I've attached) where the name is > *_FOREACH_REMOVE. We didn't add reverse removable iterators. Also, the > temp variable is the second argument. I can't think of a way of doing it > without having the externally declare the temporary variable. > A I like it and you've even done thge man page.. *_FOREACH_REMOVE however suggests that it is going to try remove something.. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
At 10:38 AM -0700 7/3/02, Terry Lambert wrote: >Julian Elischer wrote: >> TAILQ_FOREACH_REMOVABLE or TAILQ_FOREACH_SAFE >> (I prefer the first) are my suggestions for the name.) > >TAILQ_FOREACH_MODIFY ? I sense great material for a bikeshed... :-) For mine, how about: TAILQ_FOREACH_VOLATILE -- Garance Alistair Drosehn= [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Instituteor [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
Julian Elischer wrote: > TAILQ_FOREACH_REMOVABLE or TAILQ_FOREACH_SAFE > (I prefer the first) are my suggestions for the name.) TAILQ_FOREACH_MODIFY ? -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Tue, 2 Jul 2002, Ian Dowse wrote: > In message <[EMAIL PROTECTED]>, Jonathan Lemon writes: > >Essentially, this provides a traversal of the tailq that is safe > >from element removal, while being simple to drop in to those sections > >of the code that need updating, as evidenced in the patch below. > > Note that this of course is not "safe from element removal" in > general; it is just safe when you remove any element other than the > next element, whereas TAILQ_FOREACH is safe when you remove any > element other than the current one. For example it would not be > safe to call a callback that could potentially remove arbitrary > elements. > > It may be clearer in this case just to expand the macro in the code > so that it is more obvious what assumptions can be made. This would be clearer in all case :-). I think even the committer of the FOREACH macros thinks that they shouldn't be used if the list traversal has any surprises. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
I might add that phk added the itterators in 1.14, but since he's off getting married we can't ask him whether he thinks the 'safe' versions are OK.. Yeehaaa open season.. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
Julian Elischer wrote: > I would add that there is no occurance I could find in the kernel that > assumes this.. (except the bad one I mentionned before in my own code) (at > least it all runs fine with -1 put in that location on deletion), so I > must not be alone in thinking that one shouldn't rely on it.. The iterators are relatively new. It's the iterators that have introduced the assumption. -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
Julian Elischer wrote: > > On Tue, 2 Jul 2002, Terry Lambert wrote: > > > Garrett Wollman wrote: [ ... ] > > > reverted for compatibility with the other implementations of queue(3). > > I would by the way argue that the statement "The queue macros always [ ... ] For the record, Julian is responding to a statement by Garrett that I had quoted in my own response to Garrett. -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Tue, 2 Jul 2002, Julian Elischer wrote:
Having just re-read my own mail
I think I agree with jonathan now..
I think we neeed to either:
1/ augment the man page giving an example of how to do
multiple removes from a list/queue.
2/ Explain in detail why using _FOREACH()
is bad for this, and showing the alternative.
3/ Add something to the API that makes this easy to do.
designing the API addition is tricky. Jonathan's effort
was quite good, though I wonder if there is any way we can get it done
without needing the decalration of 'tmp' separatly.
(I can't think of a way).
> /*
> * Move any threads that should be suspended from the run queue
> * to the suspend queue.
> */
> TAILQ_FOREACH(from run queue) {
> if (something) {
> TAILQ_REMOVE(element from run queue)
> TAILQ_INSERT_TAIL(onto suspend queue)
> }
> }
>
> Now, at first glance, the documentation suggests this should work, even
> though we know it won't. but it doesn't crash.. it just terminates on the
> first transfer because it reaches the end of the queue.. the suspend queue
> that is..
TAILQ_FOREACH_REMOVABLE or TAILQ_FOREACH_SAFE
(I prefer the first) are my suggestions for the name.)
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Tue, 2 Jul 2002, Garrett Wollman wrote: > < >said: > > > I would by the way argue that the statement "The queue macros always > > guaranteed that traversal was safe in the presence of deletions" to be > > false. Nowhere was this guaranteed, in fact the Manual page goes to > > lengths to NOT do this.. > > I'm fairly certain that this *was* documented somewhere, at some point > in time, although I can't find it in rev. 1.1 of either the > documentation or the code. Perhaps it was in a book (Stevens 2?). I would add that there is no occurance I could find in the kernel that assumes this.. (except the bad one I mentionned before in my own code) (at least it all runs fine with -1 put in that location on deletion), so I must not be alone in thinking that one shouldn't rely on it.. > > -GAWollman > > To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
< said: > I would by the way argue that the statement "The queue macros always > guaranteed that traversal was safe in the presence of deletions" to be > false. Nowhere was this guaranteed, in fact the Manual page goes to > lengths to NOT do this.. I'm fairly certain that this *was* documented somewhere, at some point in time, although I can't find it in rev. 1.1 of either the documentation or the code. Perhaps it was in a book (Stevens 2?). -GAWollman To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Tue, 2 Jul 2002, Terry Lambert wrote:
> Garrett Wollman wrote:
> > < said:
> > > Essentially, this provides a traversal of the tailq that is safe
> > > from element removal, while being simple to drop in to those sections
> > > of the code that need updating, as evidenced in the patch below.
> >
> > The queue macros always guaranteed that traversal was safe in the
> > presence of deletions. Julian's change is erroneous and should be
> > reverted for compatibility with the other implementations of queue(3).
I would by the way argue that the statement "The queue macros always
guaranteed that traversal was safe in the presence of deletions" to be
false. Nowhere was this guaranteed, in fact the Manual page goes to
lengths to NOT do this..
note:
/* TailQ Deletion. */
while (!TAILQ_EMPTY(&head)) {
n1 = TAILQ_FIRST(&head);
TAILQ_REMOVE(&head, n1, entries);
free(n1);
}
/* Faster TailQ Deletion. */
n1 = TAILQ_FIRST(&head);
while (n1 != NULL) {
n2 = TAILQ_NEXT(n1, entries);
free(n1);
n1 = n2;
}
TAILQ_INIT(&head);
The above taken from the man page examples.
The one that killed me (why I wrote the debug code) was:
/*
* Move any threads that should be suspended from the run queue
* to the suspend queue.
*/
TAILQ_FOREACH(from run queue) {
if (something) {
TAILQ_REMOVE(element from run queue)
TAILQ_INSERT_TAIL(onto suspend queue)
}
}
Now, at first glance, the documentation suggests this should work, even
though we know it won't. but it doesn't crash.. it just terminates on the
first transfer because it reaches the end of the queue.. the suspend queue
that is..
>
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
Garrett Wollman wrote: > < said: > > Essentially, this provides a traversal of the tailq that is safe > > from element removal, while being simple to drop in to those sections > > of the code that need updating, as evidenced in the patch below. > > The queue macros always guaranteed that traversal was safe in the > presence of deletions. Julian's change is erroneous and should be > reverted for compatibility with the other implementations of queue(3). Reverting the change won't guarantee safety. In particular, in the remove case, the dereference of the "next" pointer in the removed element is highly dependent on what you do with the removed element. If you bzero it, then you will have a hell of a time dereferencing the pointer correctly. The plan only works correctly where the queue element fields themselves are type-stable (i.e. they do not get reused to link them onto another queue, and they do not get reused for other purposes, or zeroed or filled in with WITNESS or INVARIANTS test values), and if the elements themselves are allocated out of type-stable memory, as part of maintaining that guarantee. The iterator macros are ugly, in that they imply certain semantics, namely that they will work with the other macros that do element manipulations. This turns out not to be true, in the delete case. You could also come up with a failure scenario for tail insertion, or for head insertion, depending on your assumptions about whether or not an insertion while in the loop would result in the loop also doing work on an element, e.g.: loop do_work_on_element(cur_element) if (new_element > cur_element) insert(new_element) ...whether or not the work gets done on the inserted element depends on the insertion method. -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Tue, 2 Jul 2002, Jonathan Lemon wrote: > What do people think about adding the following macro to ? > (I don't care much about the name, just the functionality) > > #define TAILQ_FOREACH_TMP(var, tmp, head, field) \ > for ((var) = TAILQ_FIRST((head));\ > (var) && (((tmp) = TAILQ_NEXT((var), field)) || 1); \ > (var) = (tmp)) Certainly this is moving in the right direction.. (acknowleging the problem).. It does the job. but if someone knows to use it then they probably also know to use a temp variable themselves. It does work in that there is very little the writer can do to screw this up. The question is simply "is it waranted?" It does add complexity.. I guess it needs to be added to all the other types as well (LIST, STAILQ etc) > > Essentially, this provides a traversal of the tailq that is safe > from element removal, while being simple to drop in to those sections > of the code that need updating, as evidenced in the patch below. > -- > Jonathan To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Tue, Jul 02, 2002 at 12:58:24PM -0700, Julian Elischer wrote: > > On Tue, 2 Jul 2002, Jonathan Lemon wrote: > > > What do people think about adding the following macro to ? > > (I don't care much about the name, just the functionality) > > > > #define TAILQ_FOREACH_TMP(var, tmp, head, field) \ > > for ((var) = TAILQ_FIRST((head));\ > > (var) && (((tmp) = TAILQ_NEXT((var), field)) || 1); \ > > (var) = (tmp)) > > Certainly this is moving in the right direction.. > (acknowleging the problem).. It does the job. > but if someone knows to use it then they probably also > know to use a temp variable themselves. > > It does work in that there is very little the writer can do to screw this > up. > > The question is simply "is it waranted?" > It does add complexity.. I guess it needs to be added to all > the other types as well (LIST, STAILQ etc) Well, as Garrett pointed out, the question also is, "is this correct?" It appears that the old behavior of not modifying the list pointer may actually be part of the API (although undocumented), and one way to fix the problem is to just document the behavior. Since user programs (like libc_r) may already depend on this, perhaps the most prudent choice may be to leave the original behavior alone. -- Jonathan To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Tue, 2 Jul 2002, Jonathan Lemon wrote: > On Tue, Jul 02, 2002 at 12:58:24PM -0700, Julian Elischer wrote: > > > > On Tue, 2 Jul 2002, Jonathan Lemon wrote: > > > > > What do people think about adding the following macro to ? > > > (I don't care much about the name, just the functionality) > > > > > > #define TAILQ_FOREACH_TMP(var, tmp, head, field) \ > > > for ((var) = TAILQ_FIRST((head));\ > > > (var) && (((tmp) = TAILQ_NEXT((var), field)) || 1); \ > > > (var) = (tmp)) > > > > Certainly this is moving in the right direction.. > > (acknowleging the problem).. It does the job. > > but if someone knows to use it then they probably also > > know to use a temp variable themselves. > > > > It does work in that there is very little the writer can do to screw this > > up. > > > > The question is simply "is it waranted?" > > It does add complexity.. I guess it needs to be added to all > > the other types as well (LIST, STAILQ etc) > > Well, as Garrett pointed out, the question also is, "is this correct?" > It appears that the old behavior of not modifying the list pointer may > actually be part of the API (although undocumented), and one way to fix > the problem is to just document the behavior. > > Since user programs (like libc_r) may already depend on this, perhaps > the most prudent choice may be to leave the original behavior alone. yes. though the -1s in the kernel showed several bugs where threads were being handled as if they were on a list when they were no longer on that list. I'll leave it as part of teh debug code.. which brings up another question, Do you think I should remove the debug stuff when KSE becomes stable? Or maybe extend it to the other types. It was very useful in debugging KSE, showing the last 2 places that touched the linkages, but it may obscure teh source too much. thoughts anyone? > -- > Jonathan > To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
In message <[EMAIL PROTECTED]>, Jonathan Lemon writes: >Essentially, this provides a traversal of the tailq that is safe >from element removal, while being simple to drop in to those sections >of the code that need updating, as evidenced in the patch below. Note that this of course is not "safe from element removal" in general; it is just safe when you remove any element other than the next element, whereas TAILQ_FOREACH is safe when you remove any element other than the current one. For example it would not be safe to call a callback that could potentially remove arbitrary elements. It may be clearer in this case just to expand the macro in the code so that it is more obvious what assumptions can be made. Ian To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: additional queue macro
On Tuesday, July 2, 2002, at 10:54 AM, Jonathan Lemon wrote: > What do people think about adding the following macro to ? > (I don't care much about the name, just the functionality) > Looks great. How about TAILQ_FOREACH_SAFE? Thanks, I'm going to put this in our embedded version of queue.h here :-) Jerry Hicks To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
