From owner-cvs-all  Mon Oct 29  6:46:50 2001
Delivered-To: cvs-all@freebsd.org
Received: from fledge.watson.org (fledge.watson.org [204.156.12.50])
	by hub.freebsd.org (Postfix) with ESMTP
	id 6A19D37B405; Mon, 29 Oct 2001 06:46:21 -0800 (PST)
Received: from localhost (arr@localhost)
	by fledge.watson.org (8.11.6/8.11.5) with SMTP id f9TEkA539321;
	Mon, 29 Oct 2001 09:46:10 -0500 (EST)
	(envelope-from arr@watson.org)
Date: Mon, 29 Oct 2001 09:46:09 -0500 (EST)
From: "Andrew R. Reiter" <arr@watson.org>
To: Dag-Erling Smorgrav <des@ofug.org>
Cc: Luigi Rizzo <rizzo@aciri.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)
In-Reply-To: <xzp4roiqzrd.fsf@flood.ping.uio.no>
Message-ID: <Pine.NEB.3.96L.1011029094550.38377B-100000@fledge.watson.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
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

Bah, the point was to illustrate exactly this.  thanks for reiterating,
des.

Cheers,
Andrew

On 29 Oct 2001, 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
: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);
:
:Also, the use of the hard-coded value 0x7fffffff looks suspicious.  It
:looks like a scaling factor, which means it's probably used in several
:other places both in the kernel and in userland; it should at the very
:least be a symbolic constant (e.g. PROB_SCALE_FACTOR); at best, there
:should be a preprocessor macro that converts the dont_match_prob value
:to a double in the appropriate range.
:
:>         if (do_time) {
:>                 if (chain->timestamp) {
:>                         char timestr[30];
:>                         strcpy(timestr, ctime((time_t *)&chain->timestamp));
:>                         *strchr(timestr, '\n') = '\0';
:>                         printf("%s ", timestr);
:>                 } else {
:>                         printf("                         ");
:>                 }
:>         }
:
:This code should probably use strftime() instead of ctime(), and
:format the timestamp in ISO8601 format (YYYY-MM-DD HH:MM:SS).  It also
:makes potentially unsound assumptions about the length of the text
:generated by ctime().
:
:Both printf()s can be written more intelligently (and more obviously
:correct) using width and precision specifiers.
:
:        if (do_time) {
:                char timestr[20];
:
:                if (chain->timestamp)
:                        strftime(timestr, sizeof timestr, "%Y-%m-%d %H:%M:%S",
:                            localtime(chain->timestamp));
:                else
:                        timestr[0] = '\0';
:                printf("%-20.20s", timestr);
:        }
:
:DES
:-- 
:Dag-Erling Smorgrav - des@ofug.org
:

*-------------.................................................
| Andrew R. Reiter 
| arr@fledge.watson.org
| "It requires a very unusual mind
|   to undertake the analysis of the obvious" -- A.N. Whitehead


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