Skip site navigation (1)Skip section navigation (2)
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>
In-Reply-To: <xzp4roiqzrd.fsf@flood.ping.uio.no>
References:  <Pine.NEB.3.96L.1011029010151.33440A-100000@fledge.watson.org> <xzp4roiqzrd.fsf@flood.ping.uio.no>

next in thread | previous in thread | raw e-mail | index | archive | help
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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011029065129.B96115>