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) Message-ID: <Pine.NEB.3.96L.1011029094550.38377B-100000@fledge.watson.org> In-Reply-To: <xzp4roiqzrd.fsf@flood.ping.uio.no>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1011029094550.38377B-100000>