Re: [PATCH][2.5] introduce list_move macros (revisited)

From: Dan Aloni (da-x@gmx.net)
Date: Mon Jun 10 2002 - 10:14:50 EST


On Sun, Jun 09, 2002 at 06:42:17AM -0600, Lightweight patch manager wrote:
>
[snip]
> + */
> +#define list_move(list,head) \
> + do { __list_del(list->prev, list->next); \
> + list_add(list,head); } \
> + while(0)
> +
[snip]
>

I have a few comments about this, that were not already been said on this thread:

 * How about making it safer by assigning 'list' to something first,
   instead of letting the preprocessor evaluate it 3 times, which
   can cause problems like, when 'list' evaluates to a function call?
 * At least surround 'list' with (), as in (list)->prev, so operator
   priorities would not give you trouble.
 * Why are you adding the definitions in the end of list.h? It would
   have been more originized if it was just after list_del_init, i.e,
   along with the other list mutators.
 * The comments for the macros are not clear at all. They don't tell
   what's the purpose of these macros, which is to delete an entry from
   a list and add it to another - which is what an average user will seek
   when reading list.h.

And about macros, I quote from Documentation/SubmittingPatches:

        3) 'static inline' is better than a macro

        Static inline functions are greatly preferred over macros.
        They provide type safety, have no length limitations, no formatting
        limitations, and under gcc they are as cheap as macros.

        Macros should only be used for cases where a static inline is clearly
        suboptimal [there a few, isolated cases of this in fast paths],
        or where it is impossible to use a static inline function [such as
        string-izing].

        'static inline' is preferred over 'static __inline__', 'extern inline',
        and 'extern __inline__'.

--
Dan Aloni
da-x@gmx.net
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sat Jun 15 2002 - 22:00:18 EST