Add list_singleton to check a list has just one entry.
list_singleton is useful to check whether a list_head which
have been temporarily allocated for listing objects can be
released or not.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
include/linux/list.h | 9 +++++++++
1 file changed, 9 insertions(+)
Index: 2.6.25-rc5-mm1/include/linux/list.h
===================================================================
--- 2.6.25-rc5-mm1.orig/include/linux/list.h
+++ 2.6.25-rc5-mm1/include/linux/list.h
@@ -211,6 +211,15 @@ static inline int list_empty_careful(con
return (next == head) && (next == head->prev);
}
+/**
+ * list_singleton - tests whether a list has just one entry.
+ * @head: the list to test.
+ */
+static inline int list_singleton(const struct list_head *head)
+{
+ return !list_empty(head) && (head->next == head->prev);
+}
+
static inline void __list_splice(struct list_head *list,
struct list_head *head)
{
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: [email protected]
On Fri, 14 Mar 2008 16:40:36 -0400
Masami Hiramatsu <[email protected]> wrote:
> Add list_singleton to check a list has just one entry.
>
> list_singleton is useful to check whether a list_head which
> have been temporarily allocated for listing objects can be
> released or not.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> include/linux/list.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> Index: 2.6.25-rc5-mm1/include/linux/list.h
> ===================================================================
> --- 2.6.25-rc5-mm1.orig/include/linux/list.h
> +++ 2.6.25-rc5-mm1/include/linux/list.h
> @@ -211,6 +211,15 @@ static inline int list_empty_careful(con
> return (next == head) && (next == head->prev);
> }
>
> +/**
> + * list_singleton - tests whether a list has just one entry.
> + * @head: the list to test.
> + */
> +static inline int list_singleton(const struct list_head *head)
> +{
> + return !list_empty(head) && (head->next == head->prev);
> +}
> +
This hurts my brain.
If your usage pattern is:
struct foo {
...
struct list_head bar_list; /* A list of `struct bar's */
};
struct bar {
struct list_head list; /* Attached to foo.bar_list */
...
};
then yes, list_singleton() makes sense.
But in other usage patterns it does not:
struct foo {
struct bar *bar_list;
...
};
struct bar {
struct list_head list; /* All the other bars go here */
...
};
In the second case, emptiness is signified by foo.bar_list==NULL. And in
this case, code which does
if (foo->bar_list && list_singleton(&foo->bar_list->list))
will fail if there is a single item on the list!
The second usage pattern is uncommon and list_empty() also returns
misleading answers when list_heads are used this way.
So I guess we can proceed with your list_singleton(), but I'd just like to
flag this possible confusion, see what people think..
Andrew Morton wrote:
> If your usage pattern is:
>
> struct foo {
> ...
> struct list_head bar_list; /* A list of `struct bar's */
> };
>
> struct bar {
> struct list_head list; /* Attached to foo.bar_list */
> ...
> };
>
> then yes, list_singleton() makes sense.
>
> But in other usage patterns it does not:
>
> struct foo {
> struct bar *bar_list;
> ...
> };
>
> struct bar {
> struct list_head list; /* All the other bars go here */
> ...
> };
>
> In the second case, emptiness is signified by foo.bar_list==NULL. And in
> this case, code which does
>
> if (foo->bar_list && list_singleton(&foo->bar_list->list))
>
> will fail if there is a single item on the list!
>
> The second usage pattern is uncommon and list_empty() also returns
> misleading answers when list_heads are used this way.
I agreed. I assume that list_singleton() is used like as list_empty().
> So I guess we can proceed with your list_singleton(), but I'd just like to
> flag this possible confusion, see what people think..
Thank you,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: [email protected]
On Fri, 2008-03-14 at 18:22 -0400, Masami Hiramatsu wrote:
> Andrew Morton wrote:
> > If your usage pattern is:
> >
> > struct foo {
> > ...
> > struct list_head bar_list; /* A list of `struct bar's */
> > };
> >
> > struct bar {
> > struct list_head list; /* Attached to foo.bar_list */
> > ...
> > };
> >
> > then yes, list_singleton() makes sense.
> >
> > But in other usage patterns it does not:
> >
> > struct foo {
> > struct bar *bar_list;
> > ...
> > };
> >
> > struct bar {
> > struct list_head list; /* All the other bars go here */
> > ...
> > };
> >
> > In the second case, emptiness is signified by foo.bar_list==NULL. And in
> > this case, code which does
> >
> > if (foo->bar_list && list_singleton(&foo->bar_list->list))
> >
> > will fail if there is a single item on the list!
> >
> > The second usage pattern is uncommon and list_empty() also returns
> > misleading answers when list_heads are used this way.
>
> I agreed. I assume that list_singleton() is used like as list_empty().
>
>
> > So I guess we can proceed with your list_singleton(), but I'd just like to
> > flag this possible confusion, see what people think..
May I kindly ask to please not use the singleton name like this. It does
not implement the singleton pattern and will be a great confusion for
everybody who expects it to.
Hi Peter,
Peter Zijlstra wrote:
>>> So I guess we can proceed with your list_singleton(), but I'd just like to
>>> flag this possible confusion, see what people think..
>
> May I kindly ask to please not use the singleton name like this. It does
> not implement the singleton pattern and will be a great confusion for
> everybody who expects it to.
Indeed.
I have some other candidates.
- list_single
- list_has_one
- list_one
Could you tell me which or some other name you recommend?
Thanks,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: [email protected]
On Mon, 2008-03-17 at 11:04 -0400, Masami Hiramatsu wrote:
> Hi Peter,
>
> Peter Zijlstra wrote:
> >>> So I guess we can proceed with your list_singleton(), but I'd just like to
> >>> flag this possible confusion, see what people think..
> >
> > May I kindly ask to please not use the singleton name like this. It does
> > not implement the singleton pattern and will be a great confusion for
> > everybody who expects it to.
>
> Indeed.
>
> I have some other candidates.
> - list_single
> - list_has_one
> - list_one
>
> Could you tell me which or some other name you recommend?
I think list_has_one() or list_is_singular() are good names, they convey
they are a test for a condition by using a form of be.
Rename list_singleton to list_is_singular.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
Peter Zijlstra wrote:
> I think list_has_one() or list_is_singular() are good names, they convey
> they are a test for a condition by using a form of be.
OK, I picked up list_is_singular().
Thanks,
include/linux/list.h | 4 ++--
kernel/kprobes.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
Index: 2.6.25-rc5-mm1/include/linux/list.h
===================================================================
--- 2.6.25-rc5-mm1.orig/include/linux/list.h
+++ 2.6.25-rc5-mm1/include/linux/list.h
@@ -212,10 +212,10 @@ static inline int list_empty_careful(con
}
/**
- * list_singleton - tests whether a list has just one entry.
+ * list_is_singular - tests whether a list has just one entry.
* @head: the list to test.
*/
-static inline int list_singleton(const struct list_head *head)
+static inline int list_is_singular(const struct list_head *head)
{
return !list_empty(head) && (head->next == head->prev);
}
Index: 2.6.25-rc5-mm1/kernel/kprobes.c
===================================================================
--- 2.6.25-rc5-mm1.orig/kernel/kprobes.c
+++ 2.6.25-rc5-mm1/kernel/kprobes.c
@@ -643,7 +643,7 @@ static int __kprobes __unregister_kprobe
valid_p:
if (old_p == p ||
(old_p->pre_handler == aggr_pre_handler &&
- list_singleton(&old_p->list))) {
+ list_is_singular(&old_p->list))) {
/*
* Only probe on the hash list. Disarm only if kprobes are
* enabled - otherwise, the breakpoint would already have
@@ -679,7 +679,7 @@ static void __kprobes __unregister_kprob
module_put(mod);
}
- if (list_empty(&p->list) || list_singleton(&p->list)) {
+ if (list_empty(&p->list) || list_is_singular(&p->list)) {
if (!list_empty(&p->list)) {
/* "p" is the last child of an aggr_kprobe */
old_p = list_entry(p->list.next, struct kprobe, list);
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: [email protected]