Date: Fri, 9 Feb 2001 08:35:35 +0100 (CET) From: Luigi Rizzo <luigi@info.iet.unipi.it> To: Bruce Evans <bde@zeta.org.au> Cc: Peter Wemm <peter@netplex.com.au>, Mark Murray <mark@grondar.za>, Mike Heffner <mikeh@FreeBSD.ORG>, cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG Subject: QUEUE macros considered harmful (was Re: cvs commit: src/usr.bin/lam lam.c) Message-ID: <200102090735.IAA31362@info.iet.unipi.it> In-Reply-To: <Pine.BSF.4.21.0102091739020.12654-100000@besplex.bde.org> from Bruce Evans at "Feb 9, 2001 05:55:42 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
> __P(()) is much less visually offensive than things like LIST_FOREACH()
> which have been growing like weeds lately.
well i would say that they fall in the same category.
Personally i find these macro-extensions-to-the-language
very annoying and of questionable usefulness in many cases.
According to the manpage, we have:
12 macros for singly linked lists (SLIST)
13 macros for singly linked tail queues (STAILQ)
11 macros for [doubly-linked] lists (LIST)
15 macros for [doubly-linked] tail queues (TAILQ)
15 macros for [doubly-linked ?] circular queues (CIRCLEQ)
and apart from the number of them, the interface is not always
obvious, the manpage is not consistent, and the way they are used
is worth a prize at the Obfuscated Programming contest.
Simple example taken from the firewall code:
the manpage says that LIST_ENTRY(TYPE) "declares a structure",
but 'TYPE' is not really a type, (the real type 'struct TYPE'),
and the macro declares a type, not the actual object. The LIST
macros are then used as follows:
struct ip_fw_chain {
LIST_ENTRY(ip_fw_chain) chain;
struct ip_fw *rule;
};
...
LIST_HEAD (ip_fw_head, ip_fw_chain) ip_fw_chain;
...
chain = LIST_FIRST(&ip_fw_chain);
for (; chain; chain = LIST_NEXT(chain, chain)) {
with a wonderful mix of structures and objects and fields with the
same or similar names.
In my simple world i would have done the following:
struct ip_fw_chain {
struct ip_fw_chain *next ;
struct ip_fw *rule;
} ;
...
struct ip_fw_chain *ip_fw_chain_head ;
chain = ip_fw_chain_head ;
for (; chain ; chain = chain->next) {
which is infinitely more readable. Even the macros for inserting
into the list are totally useless and extremely obfuscating, because
there are two different ones depending on the insert position (HEAD
or in the middle) and the programmer (obviously) must know how
to scan a single-linked list.
I can admit that there are some cases where a (limited) use
of such macros can simplify life and reduce the chance of errors,
but my feeling is that we are really abusing these macros.
Thanks for bringing up the topic!
cheers
luigi
----------------------------------+-----------------------------------------
Luigi RIZZO, luigi@iet.unipi.it . ACIRI/ICSI (on leave from Univ. di Pisa)
http://www.iet.unipi.it/~luigi/ . 1947 Center St, Berkeley CA 94704
Phone (510) 666 2927 .
----------------------------------+-----------------------------------------
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200102090735.IAA31362>