Date: Wed, 27 May 2009 18:02:21 -0700 From: Zachary Loafman <zml@FreeBSD.org> To: Ed Schouten <ed@80386.nl> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r192908 - in head: share/man/man9 sys/confsys/kern sys/sys Message-ID: <20090528010220.GF3704@isilon.com> In-Reply-To: <20090527182807.GG48776@hoeg.nl> References: <200905271636.n4RGasNe003922@svn.freebsd.org> <20090527182807.GG48776@hoeg.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, May 27, 2009 at 11:28:07AM -0700, Ed Schouten wrote: > Hi Zach, > > * Zachary Loafman <zml@FreeBSD.org> wrote: > > head/sys/sys/queue.h > > This part of the patch is not in the email, so I'll just link it here: > > http://svn.freebsd.org/viewvc/base/head/sys/sys/queue.h?r1=191535&r2=192908 > > In other macros in this header file, we try to use as many _NEXT() and > _PREV() invocations as possible, to abstract the structure field names. > Maybe we could change _SWAP() macros to do the same? Ed - I started to do this, but let's run through it: > #define STAILQ_SWAP(head1, head2, type) do { \ > struct type *swap_first = STAILQ_FIRST(head1); \ > struct type **swap_last = (head1)->stqh_last; \ > STAILQ_FIRST(head1) = STAILQ_FIRST(head2); \ > (head1)->stqh_last = (head2)->stqh_last; \ > STAILQ_FIRST(head2) = swap_first; \ > (head2)->stqh_last = swap_last; \ > if (STAILQ_EMPTY(head1)) \ > (head1)->stqh_last = &STAILQ_FIRST(head1); \ > if (STAILQ_EMPTY(head2)) \ > (head2)->stqh_last = &STAILQ_FIRST(head2); \ > } while (0) There is no macro that references just ->stqh_last. As it's internal to the inner workings, I'm not sure there needs to be. The others are macro-ized already. > #define LIST_SWAP(head1, head2, type, field) do { \ > struct type *swap_tmp = LIST_FIRST((head1)); \ > LIST_FIRST((head1)) = LIST_FIRST((head2)); \ > LIST_FIRST((head2)) = swap_tmp; \ > if ((swap_tmp = LIST_FIRST((head1))) != NULL) \ > swap_tmp->field.le_prev = &LIST_FIRST((head1)); \ > if ((swap_tmp = LIST_FIRST((head2))) != NULL) \ > swap_tmp->field.le_prev = &LIST_FIRST((head2)); \ > } while (0) Again, there's no macro for .le_prev. Everything else is macro-ized. > #define TAILQ_SWAP(head1, head2, type, field) do { \ > struct type *swap_first = (head1)->tqh_first; \ > struct type **swap_last = (head1)->tqh_last; \ > (head1)->tqh_first = (head2)->tqh_first; \ > (head1)->tqh_last = (head2)->tqh_last; \ > (head2)->tqh_first = swap_first; \ > (head2)->tqh_last = swap_last; \ > if ((swap_first = (head1)->tqh_first) != NULL) \ > swap_first->field.tqe_prev = &(head1)->tqh_first; \ > else \ > (head1)->tqh_last = &(head1)->tqh_first; \ > if ((swap_first = (head2)->tqh_first) != NULL) \ > swap_first->field.tqe_prev = &(head2)->tqh_first; \ > else \ > (head2)->tqh_last = &(head2)->tqh_first; \ > } while (0) This is the only one where we could throw some macro replacements in. But consider this: > (head1)->tqh_first = (head2)->tqh_first; \ > (head1)->tqh_last = (head2)->tqh_last; \ versus > TAILQ_FIRST(head1) = TAILQ_FIRST(head2); \ > (head1)->tqh_last = (head2)->tqh_last; \ I personally find the first form clearer to skim through visually. The second form moves 'head1' and 'head2' around. It's not like it's hard to follow, but I think the first is clearer to read. I don't think the TAILQ_FIRST buys you much here. -- Zach Loafman | Staff Engineer | Isilon Systems
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090528010220.GF3704>