From owner-cvs-all  Mon Dec 31 16:52:37 2001
Delivered-To: cvs-all@freebsd.org
Received: from kayak.xcllnt.net (209-128-86-226.bayarea.net [209.128.86.226])
	by hub.freebsd.org (Postfix) with ESMTP
	id 91CBF37B41F; Mon, 31 Dec 2001 16:52:27 -0800 (PST)
Received: from dhcp01.pn.xcllnt.net (dhcp01.pn.xcllnt.net [192.168.4.201])
	by kayak.xcllnt.net (8.11.6/8.11.4) with ESMTP id g010qRQ66520;
	Mon, 31 Dec 2001 16:52:27 -0800 (PST)
	(envelope-from marcel@kayak.pn.xcllnt.net)
Received: (from marcel@localhost)
	by dhcp01.pn.xcllnt.net (8.11.6/8.11.6) id g010qX500503;
	Mon, 31 Dec 2001 16:52:33 -0800 (PST)
	(envelope-from marcel)
Date: Mon, 31 Dec 2001 16:52:33 -0800
From: Marcel Moolenaar <marcel@xcllnt.net>
To: Julian Elischer <julian@elischer.org>
Cc: John Baldwin <jhb@FreeBSD.org>,
	Poul-Henning Kamp <phk@critter.freebsd.dk>, cvs-all@FreeBSD.org,
	cvs-committers@FreeBSD.org, Julian Elischer <julian@FreeBSD.org>,
	Greg Lehey <grog@FreeBSD.org>
Subject: Re: cvs commit: src/share/man/man3 queue.3
Message-ID: <20011231165233.A408@dhcp01.pn.xcllnt.net>
References: <XFMail.011231123912.jhb@FreeBSD.org> <Pine.BSF.4.21.0112311328270.94344-100000@InterJet.elischer.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Pine.BSF.4.21.0112311328270.94344-100000@InterJet.elischer.org>
User-Agent: Mutt/1.3.21i
Sender: owner-cvs-all@FreeBSD.ORG
Precedence: bulk
List-ID: <cvs-all.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20cvs-all>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20cvs-all>
X-Loop: FreeBSD.ORG

On Mon, Dec 31, 2001 at 01:38:53PM -0800, Julian Elischer wrote:
> > > 
> > > _FOREACH() is for the plain & simple traversal of a queue(3) stucture,
> > > anything more should explicitly state intent.
> > 
> > I would agree except for code like this:
> > 
> >         TAILQ_FOREACH(p, &list, p_next)
> >                 if (some_test_on(p))
> >                         break;
> >         if (p != NULL) {
> >                 /* Found one, now do something. */
> >         }
> 
> This is the case I'm using it for.
> except I'm doing:
>          TAILQ_FOREACH(p, &list, p_next)
>                  if (some_test_on(p))
>                          break;
>          if (p != NULL) {
>                  /* Found it, now do something. */
>          }
> lots of code that MAY clear p
>          if (p == NULL) {
>                  /* didn't find it, or it was disqualified */
>          }

I'd consider this a misuse of _FOREACH, because the something that's
being done is definitely not done for each p. Even some_test_on() is
not performed on each p. I'm with phk here.

Note that if it's simply too easy to use _FOREACH in these cases, we
may want to consider creating a more suitable abstraction, like:

	TAILQ_UNTIL(p, &list, p_next)
		some_test_on(p);
	if (p == 0)
		return ENOTFOUND;
	/* do something */
	return 0;

Just a thought.

-- 
 Marcel Moolenaar	  USPA: A-39004		 marcel@xcllnt.net

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message