From owner-cvs-all  Mon Oct 29  6:55: 7 2001
Delivered-To: cvs-all@freebsd.org
Received: from iguana.aciri.org (iguana.aciri.org [192.150.187.36])
	by hub.freebsd.org (Postfix) with ESMTP
	id 2F5F737B405; Mon, 29 Oct 2001 06:55:03 -0800 (PST)
Received: (from rizzo@localhost)
	by iguana.aciri.org (8.11.3/8.11.1) id f9TEpTL98991;
	Mon, 29 Oct 2001 06:51:29 -0800 (PST)
	(envelope-from rizzo)
Date: Mon, 29 Oct 2001 06:51:29 -0800
From: Luigi Rizzo <rizzo@aciri.org>
To: Dag-Erling Smorgrav <des@ofug.org>
Cc: "Andrew R. Reiter" <arr@watson.org>,
	Josef Karthauser <joe@FreeBSD.org>, cvs-committers@FreeBSD.org,
	cvs-all@FreeBSD.org
Subject: Re: ipfw.c -- (was: cvs commit: src/sys/netinet ip_fw.h)
Message-ID: <20011029065129.B96115@iguana.aciri.org>
References: <Pine.NEB.3.96L.1011029010151.33440A-100000@fledge.watson.org> <xzp4roiqzrd.fsf@flood.ping.uio.no>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <xzp4roiqzrd.fsf@flood.ping.uio.no>
User-Agent: Mutt/1.3.23i
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, Oct 29, 2001 at 03:36:38PM +0100, Dag-Erling Smorgrav wrote:
> "Andrew R. Reiter" <arr@watson.org> writes:
> >         if (chain->fw_flg & IP_FW_F_RND_MATCH) {
> >                 double d = 1.0 * chain->dont_match_prob;
> >                 d = 1 - (d / 0x7fffffff);
> >                 printf("prob %f ", d);
> >         }
> 
> There is no need for a temporary variable here.  The probability
> computation can be greatly simplified.  BTW, the format string should

Sure there is no need for a temp variable, and sure the code could
be written in a better way, using macros for scale factors etc.

But the code you suggest is _wrong_:
chain->dont_match_prob is an int, and the compiler does an integer
division (which is not what you want) unless you properly instruct
it to  do a floating point calculation (with a cast, a multiplication
for 1.0, or whatever you like).

By the time you put in the cast, the line becomes so long that it
probably becomes worthwhile (for readability purposes) to do the
calculation outside the printf, etc. etc.

Next time, how about thinking a couple of seconds before lecturing people :)

	cheers
	luigi

> specify the number of decimals (e.g. "%.3f").
> 
>         if (chain->fw_flg & IP_FW_F_RND_MATCH)
>                 printf("prob %.3f ",
>                     1.0 - chain->dont_match_prob / 0x7fffffff);
> 

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